Skip to content

Commit

Permalink
Close FileEntries of cached files in ModuleManager::addModule().
Browse files Browse the repository at this point in the history
While investigating why LLDB (which can build hundreds of clang
modules during one debug session) was getting "too many open files"
errors, I found that most of them are .pcm files that are kept open by
ModuleManager. Pretty much all of the open file dscriptors are
FileEntries that are refering to `.pcm` files for which a buffer
already exists in a CompilerInstance's PCMCache.

Before PCMCache was added it was necessary to hold on to open file
descriptors to ensure that all ModuleManagers using the same
FileManager read the a consistent version of a given `.pcm` file on
disk, even when a concurrent clang process overwrites the file halfway
through. The PCMCache makes this practice unnecessary, since it caches
the entire contents of a `.pcm` file, while the FileManager caches all
the stat() information.

This patch adds a call to FileEntry::closeFile() to the path where a
Buffer has already been created. This is necessary because even for a
freshly written `.pcm` file the file is stat()ed once immediately
after writing to generate a FileEntry in the FileManager. Because a
freshly-generated file's contents is stored in the PCMCache, it is
fine to close the file immediately thereafter.  The second change this
patch makes is to set the `ShouldClose` flag to true when reading a
`.pcm` file into the PCMCache for the first time.

[For reference, in 1 Clang instance there is
     - 1 FileManager and
     - n ModuleManagers with
     - n PCMCaches.]

rdar://problem/40906753

Differential Revision: https://reviews.llvm.org/D50870

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340188 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
adrian-prantl committed Aug 20, 2018
1 parent ae679b0 commit 6f4f824
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions lib/Serialization/ModuleManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,21 +161,24 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
// The buffer was already provided for us.
NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer));
// Since the cached buffer is reused, it is safe to close the file
// descriptor that was opened while stat()ing the PCM in
// lookupModuleFile() above, it won't be needed any longer.
Entry->closeFile();
} else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) {
NewModule->Buffer = Buffer;
// As above, the file descriptor is no longer needed.
Entry->closeFile();
} else {
// Open the AST file.
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
if (FileName == "-") {
Buf = llvm::MemoryBuffer::getSTDIN();
} else {
// Leave the FileEntry open so if it gets read again by another
// ModuleManager it must be the same underlying file.
// FIXME: Because FileManager::getFile() doesn't guarantee that it will
// give us an open file, this may not be 100% reliable.
// Get a buffer of the file and close the file descriptor when done.
Buf = FileMgr.getBufferForFile(NewModule->File,
/*IsVolatile=*/false,
/*ShouldClose=*/false);
/*ShouldClose=*/true);
}

if (!Buf) {
Expand Down

0 comments on commit 6f4f824

Please sign in to comment.