[Support] Remove output file checks from LockFileManager (llvm#130395)

Currently, `LockFileManager` assumes the owner of the lock file creates
an output file. This is problematic for at least three reasons:

1. It is orthogonal to the main purpose of this class - mutual
exclusion. This makes creating an alternative implementation more
complicated than it needs to be.

2. Some clients (like the upstream `AMDGPUSplitModule.cpp` file) assume
the output file is not necessary. The owner of the lock file does not
write the file expected by `LockFileManager` and the processes waiting
for the non-owned lock file to be unlocked therefore assume the owner
has died. This means that the work gets repeated by each waiting
process, serially.

3. The documentation makes it sound like successfully waiting for a
non-owned lock file guarantees the output file to be present on the file
system. Implicitly-built modules rely on this. However, the module file
may disappear between `LockFileManager` performing the check and the
compiler loading the module (for example due to module cache pruning
with short intervals, or intervention from outside of Clang). The
compiler assumes this cannot happen, and fails the build if it does.

This PR solves this situation by removing the check, reflecting that in
the `LockFileManager` documentation, and fixing the time-of-check
time-of-use bug in implicit modules.

(cherry picked from commit f90aa41)

Author: jansvoboda11

clang/lib/Frontend/CompilerInstance.cpp

@@ -1557,7 +1557,7 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance,
                                       SourceLocation ImportLoc,
                                       SourceLocation ModuleNameLoc,
                                       Module *Module, StringRef ModuleFileName,
-                                      bool *OutOfDate) {
+                                      bool *OutOfDate, bool *Missing) {
   DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();
 
   unsigned ModuleLoadCapabilities = ASTReader::ARR_Missing;
@@ -1578,6 +1578,12 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance,
     return false;
   }
 
+  // The caller wants to handle missing module files.
+  if (Missing && ReadResult == ASTReader::Missing) {
+    *Missing = true;
+    return false;
+  }
+
   // The ASTReader didn't diagnose the error, so conservatively report it.
   if (ReadResult == ASTReader::Missing || !Diags.hasErrorOccurred())
     Diags.Report(ModuleNameLoc, diag::err_module_not_built)
@@ -1603,7 +1609,7 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance,
 
   return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
                                    Module, ModuleFileName,
-                                   /*OutOfDate=*/nullptr);
+                                   /*OutOfDate=*/nullptr, /*Missing=*/nullptr);
 }
 
 /// Compile a module in a separate compiler instance and read the AST,
@@ -1668,15 +1674,17 @@ static bool compileModuleAndReadASTBehindLock(
 
     // Read the module that was just written by someone else.
     bool OutOfDate = false;
+    bool Missing = false;
     if (readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
-                                  Module, ModuleFileName, &OutOfDate))
+                                  Module, ModuleFileName, &OutOfDate, &Missing))
       return true;
-    if (!OutOfDate)
+    if (!OutOfDate && !Missing)
       return false;
 
-    // The module may be out of date in the presence of file system races,
-    // or if one of its imports depends on header search paths that are not
-    // consistent with this ImportingInstance.  Try again...
+    // The module may be missing or out of date in the presence of file system
+    // races. It may also be out of date if one of its imports depends on header
+    // search paths that are not consistent with this ImportingInstance.
+    // Try again...
   }
 }
 

llvm/include/llvm/Support/LockFileManager.h

@@ -16,14 +16,13 @@
 namespace llvm {
 class StringRef;
 
-/// Class that manages the creation of a lock file to aid
-/// implicit coordination between different processes.
+/// Class that manages the creation of a lock file to aid implicit coordination
+/// between different processes.
 ///
-/// The implicit coordination works by creating a ".lock" file alongside
-/// the file that we're coordinating for, using the atomicity of the file
-/// system to ensure that only a single process can create that ".lock" file.
-/// When the lock file is removed, the owning process has finished the
-/// operation.
+/// The implicit coordination works by creating a ".lock" file, using the
+/// atomicity of the file system to ensure that only a single process can create
+/// that ".lock" file. When the lock file is removed, the owning process has
+/// finished the operation.
 class LockFileManager {
 public:
   /// Describes the state of a lock file.

llvm/lib/Support/LockFileManager.cpp

@@ -307,12 +307,8 @@ LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
   while (Backoff.waitForNextAttempt()) {
     // FIXME: implement event-based waiting
     if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist) ==
-        errc::no_such_file_or_directory) {
-      // If the original file wasn't created, somone thought the lock was dead.
-      if (!sys::fs::exists(FileName))
-        return Res_OwnerDied;
+        errc::no_such_file_or_directory)
       return Res_Success;
-    }
 
     // If the process owning the lock died without cleaning up, just bail out.
     if (!processStillExecuting((*Owner).first, (*Owner).second))