Merge pull request #29123 from nkcsgexi/reapply-file-lock

Re-apply "ModuleInterface: lock .swiftinterface while generating module cache"

Author: nkcsgexi

include/swift/AST/DiagnosticsFrontend.def

@@ -343,6 +343,12 @@ ERROR(unknown_forced_module_loading_mode,none,
       "unknown value for SWIFT_FORCE_MODULE_LOADING variable: '%0'",
       (StringRef))
 
+REMARK(interface_file_lock_failure,none,
+      "could not acquire lock file for module interface '%0'", (StringRef))
+
+REMARK(interface_file_lock_timed_out,none,
+      "timed out waiting to acquire lock file for module interface '%0'", (StringRef))
+
 #ifndef DIAG_NO_UNDEF
 # if defined(DIAG)
 #  undef DIAG

lib/Frontend/ModuleInterfaceBuilder.cpp

@@ -36,6 +36,7 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/StringSaver.h"
+#include "llvm/Support/LockFileManager.h"
 
 using namespace swift;
 using FileDependency = SerializationOptions::FileDependency;
@@ -240,7 +241,7 @@ bool ModuleInterfaceBuilder::collectDepsForSerialization(
   return false;
 }
 
-bool ModuleInterfaceBuilder::buildSwiftModule(
+bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
     StringRef OutPath, bool ShouldSerializeDeps,
     std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer) {
   bool SubError = false;
@@ -384,3 +385,71 @@ bool ModuleInterfaceBuilder::buildSwiftModule(
   });
   return !RunSuccess || SubError;
 }
+
+bool ModuleInterfaceBuilder::buildSwiftModule(StringRef OutPath,
+                                              bool ShouldSerializeDeps,
+                          std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
+                          llvm::function_ref<void()> RemarkRebuild) {
+
+  while (1) {
+  // Attempt to lock the interface file. Only one process is allowed to build
+  // module from the interface so we don't consume too much memory when multiple
+  // processes are doing the same.
+  // FIXME: We should surface the module building step to the build system so
+  // we don't need to synchronize here.
+  llvm::LockFileManager Locked(interfacePath);
+  switch (Locked) {
+  case llvm::LockFileManager::LFS_Error:{
+    // ModuleInterfaceBuilder takes care of correctness and locks are only
+    // necessary for performance. Fallback to building the module in case of any lock
+    // related errors.
+    if (RemarkRebuild) {
+      diags.diagnose(SourceLoc(), diag::interface_file_lock_failure,
+                     interfacePath);
+    }
+    // Clear out any potential leftover.
+    Locked.unsafeRemoveLockFile();
+    LLVM_FALLTHROUGH;
+  }
+  case llvm::LockFileManager::LFS_Owned: {
+    if (RemarkRebuild) {
+      RemarkRebuild();
+    }
+    return buildSwiftModuleInternal(OutPath, ShouldSerializeDeps, ModuleBuffer);
+  }
+  case llvm::LockFileManager::LFS_Shared: {
+    // Someone else is responsible for building the module. Wait for them to
+    // finish.
+    switch (Locked.waitForUnlock()) {
+    case llvm::LockFileManager::Res_Success: {
+      // This process may have a different module output path. If the other
+      // process doesn't build the interface to this output path, we should try
+      // building ourselves.
+      auto bufferOrError = llvm::MemoryBuffer::getFile(OutPath);
+      if (!bufferOrError)
+        continue;
+      if (ModuleBuffer)
+        *ModuleBuffer = std::move(bufferOrError.get());
+      return false;
+    }
+    case llvm::LockFileManager::Res_OwnerDied: {
+      continue; // try again to get the lock.
+    }
+    case llvm::LockFileManager::Res_Timeout: {
+      // Since ModuleInterfaceBuilder takes care of correctness, we try waiting for
+      // another process to complete the build so swift does not do it done
+      // twice. If case of timeout, build it ourselves.
+      if (RemarkRebuild) {
+        diags.diagnose(SourceLoc(), diag::interface_file_lock_timed_out,
+                       interfacePath);
+      }
+      // Clear the lock file so that future invocations can make progress.
+      Locked.unsafeRemoveLockFile();
+      continue;
+    }
+    }
+    break;
+  }
+  }
+  }
+}

lib/Frontend/ModuleInterfaceBuilder.h

@@ -67,6 +67,8 @@ class ModuleInterfaceBuilder {
       version::Version &Vers, llvm::StringSaver &SubArgSaver,
       SmallVectorImpl<const char *> &SubArgs);
 
+  bool buildSwiftModuleInternal(StringRef OutPath, bool ShouldSerializeDeps,
+                                std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
 public:
   ModuleInterfaceBuilder(SourceManager &sourceMgr, DiagnosticEngine &diags,
                             const SearchPathOptions &searchPathOpts,
@@ -102,7 +104,8 @@ class ModuleInterfaceBuilder {
   }
 
   bool buildSwiftModule(StringRef OutPath, bool ShouldSerializeDeps,
-                        std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer);
+                        std::unique_ptr<llvm::MemoryBuffer> *ModuleBuffer,
+                        llvm::function_ref<void()> RemarkRebuild = nullptr);
 };
 
 } // end namespace swift

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -950,13 +950,11 @@ class ModuleInterfaceLoaderImpl {
     std::unique_ptr<llvm::MemoryBuffer> moduleBuffer;
 
     // We didn't discover a module corresponding to this interface.
-
     // Diagnose that we didn't find a loadable module, if we were asked to.
-    if (remarkOnRebuildFromInterface) {
+    auto remarkRebuild = [&]() {
       rebuildInfo.diagnose(ctx, diagnosticLoc, moduleName,
                            interfacePath);
-    }
-
+    };
     // If we found an out-of-date .swiftmodule, we still want to add it as
     // a dependency of the .swiftinterface. That way if it's updated, but
     // the .swiftinterface remains the same, we invalidate the cache and
@@ -966,7 +964,9 @@ class ModuleInterfaceLoaderImpl {
       builder.addExtraDependency(modulePath);
 
     if (builder.buildSwiftModule(cachedOutputPath, /*shouldSerializeDeps*/true,
-                                 &moduleBuffer))
+                                 &moduleBuffer,
+                                 remarkOnRebuildFromInterface ? remarkRebuild:
+                                   llvm::function_ref<void()>()))
       return std::make_error_code(std::errc::invalid_argument);
 
     assert(moduleBuffer &&

test/Driver/lock_interface.swift

@@ -0,0 +1,13 @@
+// RUN: %empty-directory(%t)
+// RUN: echo 'public func foo() {}' > %t/Foo.swift
+// RUN: %target-swift-frontend-typecheck -emit-module-interface-path %t/Foo.swiftinterface %t/Foo.swift -enable-library-evolution
+// RUN: touch %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift
+// RUN: echo 'import Foo' > %t/file-01.swift
+// RUN: echo 'import Foo' > %t/file-02.swift
+// RUN: echo 'import Foo' > %t/file-03.swift
+// RUN: %target-swiftc_driver -j20 %t/main.swift %t/file-01.swift %t/file-02.swift %t/file-03.swift -I %t -Xfrontend -Rmodule-interface-rebuild &> %t/result.txt
+// RUN: %FileCheck %s  -check-prefix=CHECK-REBUILD < %t/result.txt
+
+// Ensure we only build Foo module once from the interface
+// CHECK-REBUILD: rebuilding module 'Foo' from interface
+// CHECK-REBUILD-NOT: rebuilding module 'Foo' from interface