[PATCH] D114966: [clang][deps] Split filesystem caches

[Jan Svoboda via Phabricator via llvm-commits]

jansvoboda11 marked 3 inline comments as done.
jansvoboda11 added inline comments.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:25-31
   llvm::ErrorOr<llvm::vfs::Status> Stat = (*MaybeFile)->status();
   if (!Stat)
     return Stat.getError();
 
   llvm::vfs::File &F = **MaybeFile;
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MaybeBuffer =
       F.getBuffer(Stat->getName());
----------------
dexonsmith wrote:
> Is there a potential (already existing) race condition here? Can't the file change between the stat and opening the buffer?
> 
> Seems like either:
> - The `Stat` should be updated to have the observed size of the buffer.
> - An error should be returned if the size doesn't match.
> - The stat and/or read should be retried until they match.
I think that's right. I left a detailed FIXME in the code calling `read()` and would like to tackle that in a follow up. Would that be fine?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114966/new/

https://reviews.llvm.org/D114966