[PATCH] D114966: [clang][deps] Split stat and file content caches

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith 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());
----------------
jansvoboda11 wrote:
> 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?
Yup, doing it in a separate commit makes sense. I suggest taking the first option, since it's the simplest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114966