From: Adrian Prantl <aprantl@apple.com>
Date: Mon, 20 Aug 2018 17:10:27 +0000 (+0000)
Subject: Close FileEntries of cached files in ModuleManager::addModule().
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6f4f824d5674b1ab5276cd5e3cd93bb7ede784ca;p=clang

Close FileEntries of cached files in ModuleManager::addModule().

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
---

diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp
index 57ebaca10c..fccfa88ab9 100644
--- a/lib/Serialization/ModuleManager.cpp
+++ b/lib/Serialization/ModuleManager.cpp
@@ -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) {