[clang] [clang][modules][deps] Add mutex as an alternative to file lock (PR #129751)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 5 16:56:58 PST 2025


https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/129751

>From 5f119a3a4ae8642d6ab0498c1cf6b39d5099c835 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 27 Feb 2025 15:23:18 -0800
Subject: [PATCH 1/5] [clang][modules][deps] Add mutex as an alternative to
 file lock

---
 .../include/clang/Frontend/CompilerInstance.h | 13 ++-
 .../clang/Serialization/ModuleCacheLock.h     | 31 +++++++
 .../DependencyScanningService.h               |  7 ++
 .../DependencyScanning/ModuleCacheMutexLock.h | 31 +++++++
 clang/lib/Frontend/CompilerInstance.cpp       | 47 ++++++-----
 clang/lib/Serialization/CMakeLists.txt        |  1 +
 clang/lib/Serialization/ModuleCacheLock.cpp   | 60 ++++++++++++++
 .../Tooling/DependencyScanning/CMakeLists.txt |  1 +
 .../DependencyScanningWorker.cpp              |  7 +-
 .../ModuleCacheMutexLock.cpp                  | 81 +++++++++++++++++++
 10 files changed, 254 insertions(+), 25 deletions(-)
 create mode 100644 clang/include/clang/Serialization/ModuleCacheLock.h
 create mode 100644 clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
 create mode 100644 clang/lib/Serialization/ModuleCacheLock.cpp
 create mode 100644 clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp

diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 8b539dfc92960..bede160991443 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -53,6 +53,7 @@ class FileManager;
 class FrontendAction;
 class InMemoryModuleCache;
 class Module;
+class ModuleCacheLock;
 class Preprocessor;
 class Sema;
 class SourceManager;
@@ -96,9 +97,12 @@ class CompilerInstance : public ModuleLoader {
   /// The source manager.
   IntrusiveRefCntPtr<SourceManager> SourceMgr;
 
-  /// The cache of PCM files.
+  /// The local in-memory cache of PCM files.
   IntrusiveRefCntPtr<InMemoryModuleCache> ModuleCache;
 
+  /// The lock managing the global cache of PCM files.
+  std::shared_ptr<ModuleCacheLock> ModuleCacheLck;
+
   /// The preprocessor.
   std::shared_ptr<Preprocessor> PP;
 
@@ -209,7 +213,8 @@ class CompilerInstance : public ModuleLoader {
   explicit CompilerInstance(
       std::shared_ptr<PCHContainerOperations> PCHContainerOps =
           std::make_shared<PCHContainerOperations>(),
-      InMemoryModuleCache *SharedModuleCache = nullptr);
+      InMemoryModuleCache *SharedModuleCache = nullptr,
+      std::shared_ptr<ModuleCacheLock> ModuleCacheLck = nullptr);
   ~CompilerInstance() override;
 
   /// @name High-Level Operations
@@ -897,6 +902,10 @@ class CompilerInstance : public ModuleLoader {
   void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS);
 
   InMemoryModuleCache &getModuleCache() const { return *ModuleCache; }
+
+  std::shared_ptr<ModuleCacheLock> getModuleCacheLockPtr() const {
+    return ModuleCacheLck;
+  }
 };
 
 } // end namespace clang
diff --git a/clang/include/clang/Serialization/ModuleCacheLock.h b/clang/include/clang/Serialization/ModuleCacheLock.h
new file mode 100644
index 0000000000000..9435735b1a1bd
--- /dev/null
+++ b/clang/include/clang/Serialization/ModuleCacheLock.h
@@ -0,0 +1,31 @@
+#ifndef LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H
+#define LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/Support/LockFileManager.h"
+
+namespace clang {
+enum class LockResult { Owned, Shared, Error };
+enum class WaitForUnlockResult { Success, OwnerDied, Timeout };
+
+class ModuleCacheLockManager {
+public:
+  virtual operator LockResult() const = 0;
+  virtual WaitForUnlockResult waitForUnlock() = 0;
+  virtual void unsafeRemoveLock() = 0;
+  virtual std::string getErrorMessage() const = 0;
+  virtual ~ModuleCacheLockManager() = default;
+};
+
+class ModuleCacheLock {
+public:
+  virtual void prepareLock(StringRef ModuleFilename) = 0;
+  virtual std::unique_ptr<ModuleCacheLockManager>
+  tryLock(StringRef ModuleFilename) = 0;
+  virtual ~ModuleCacheLock() = default;
+};
+
+std::shared_ptr<ModuleCacheLock> getModuleCacheFileLock();
+} // namespace clang
+
+#endif
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index f002f8645d3f6..f6914b90ac67e 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 
+#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
 #include "llvm/ADT/BitmaskEnum.h"
 
@@ -95,6 +96,10 @@ class DependencyScanningService {
     return SharedCache;
   }
 
+  ModuleCacheMutexes &getSharedModuleCacheMutexes() {
+    return ModuleCacheMutexes;
+  }
+
 private:
   const ScanningMode Mode;
   const ScanningOutputFormat Format;
@@ -106,6 +111,8 @@ class DependencyScanningService {
   const bool TraceVFS;
   /// The global file system cache.
   DependencyScanningFilesystemSharedCache SharedCache;
+  /// The global module cache mutexes.
+  ModuleCacheMutexes ModuleCacheMutexes;
 };
 
 } // end namespace dependencies
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
new file mode 100644
index 0000000000000..312972b26702c
--- /dev/null
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
@@ -0,0 +1,31 @@
+#ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H
+#define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H
+
+#include "clang/Serialization/ModuleCacheLock.h"
+#include "llvm/ADT/StringMap.h"
+
+#include <condition_variable>
+
+namespace clang {
+namespace tooling {
+namespace dependencies {
+struct ModuleCacheMutexWrapper {
+  std::mutex Mutex;
+  std::condition_variable CondVar;
+  bool Done = false;
+
+  ModuleCacheMutexWrapper() = default;
+};
+
+struct ModuleCacheMutexes {
+  std::mutex Mutex;
+  llvm::StringMap<std::shared_ptr<ModuleCacheMutexWrapper>> Map;
+};
+
+std::shared_ptr<ModuleCacheLock>
+getModuleCacheMutexLock(ModuleCacheMutexes &Mutexes);
+} // namespace dependencies
+} // namespace tooling
+} // namespace clang
+
+#endif
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c11c857ea0606..3d02d42032e21 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -39,6 +39,7 @@
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/GlobalModuleIndex.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
+#include "clang/Serialization/ModuleCacheLock.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -66,11 +67,14 @@ using namespace clang;
 
 CompilerInstance::CompilerInstance(
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
-    InMemoryModuleCache *SharedModuleCache)
+    InMemoryModuleCache *SharedModuleCache,
+    std::shared_ptr<ModuleCacheLock> ModuleCacheLck)
     : ModuleLoader(/* BuildingModule = */ SharedModuleCache),
       Invocation(new CompilerInvocation()),
       ModuleCache(SharedModuleCache ? SharedModuleCache
                                     : new InMemoryModuleCache),
+      ModuleCacheLck(ModuleCacheLck ? ModuleCacheLck
+                                    : getModuleCacheFileLock()),
       ThePCHContainerOperations(std::move(PCHContainerOps)) {}
 
 CompilerInstance::~CompilerInstance() {
@@ -1227,7 +1231,8 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
   // CompilerInstance::CompilerInstance is responsible for finalizing the
   // buffers to prevent use-after-frees.
   CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(),
-                            &ImportingInstance.getModuleCache());
+                            &ImportingInstance.getModuleCache(),
+                            ImportingInstance.getModuleCacheLockPtr());
   auto &Inv = *Invocation;
   Instance.setInvocation(std::move(Invocation));
 
@@ -1471,47 +1476,45 @@ static bool compileModuleAndReadASTBehindLock(
   Diags.Report(ModuleNameLoc, diag::remark_module_lock)
       << ModuleFileName << Module->Name;
 
-  // FIXME: have LockFileManager return an error_code so that we can
-  // avoid the mkdir when the directory already exists.
-  StringRef Dir = llvm::sys::path::parent_path(ModuleFileName);
-  llvm::sys::fs::create_directories(Dir);
+  auto Lock = ImportingInstance.getModuleCacheLockPtr();
+  Lock->prepareLock(ModuleFileName);
 
   while (true) {
-    llvm::LockFileManager Locked(ModuleFileName);
-    switch (Locked) {
-    case llvm::LockFileManager::LFS_Error:
+    auto Locked = Lock->tryLock(ModuleFileName);
+    switch (*Locked) {
+    case LockResult::Error:
       // ModuleCache takes care of correctness and locks are only necessary for
       // performance. Fallback to building the module in case of any lock
       // related errors.
       Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
-          << Module->Name << Locked.getErrorMessage();
+          << Module->Name << Locked->getErrorMessage();
       // Clear out any potential leftover.
-      Locked.unsafeRemoveLockFile();
+      Locked->unsafeRemoveLock();
       [[fallthrough]];
-    case llvm::LockFileManager::LFS_Owned:
+    case LockResult::Owned:
       // We're responsible for building the module ourselves.
       return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
                                          ModuleNameLoc, Module, ModuleFileName);
 
-    case llvm::LockFileManager::LFS_Shared:
+    case LockResult::Shared:
       break; // The interesting case.
     }
 
     // Someone else is responsible for building the module. Wait for them to
     // finish.
-    switch (Locked.waitForUnlock()) {
-    case llvm::LockFileManager::Res_Success:
+    switch (Locked->waitForUnlock()) {
+    case WaitForUnlockResult::Success:
       break; // The interesting case.
-    case llvm::LockFileManager::Res_OwnerDied:
+    case WaitForUnlockResult::OwnerDied:
       continue; // try again to get the lock.
-    case llvm::LockFileManager::Res_Timeout:
-      // Since ModuleCache takes care of correctness, we try waiting for
-      // another process to complete the build so clang does not do it done
-      // twice. If case of timeout, build it ourselves.
+    case WaitForUnlockResult::Timeout:
+      // Since the InMemoryModuleCache takes care of correctness, we try waiting
+      // for someone else to complete the build so that it does not happen
+      // twice. In case of timeout, build it ourselves.
       Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
           << Module->Name;
-      // Clear the lock file so that future invocations can make progress.
-      Locked.unsafeRemoveLockFile();
+      // Remove the lock so that future invocations can make progress.
+      Locked->unsafeRemoveLock();
       continue;
     }
 
diff --git a/clang/lib/Serialization/CMakeLists.txt b/clang/lib/Serialization/CMakeLists.txt
index b1fc0345047f2..acbd8669caaed 100644
--- a/clang/lib/Serialization/CMakeLists.txt
+++ b/clang/lib/Serialization/CMakeLists.txt
@@ -18,6 +18,7 @@ add_clang_library(clangSerialization
   GeneratePCH.cpp
   GlobalModuleIndex.cpp
   InMemoryModuleCache.cpp
+  ModuleCacheLock.cpp
   ModuleFile.cpp
   ModuleFileExtension.cpp
   ModuleManager.cpp
diff --git a/clang/lib/Serialization/ModuleCacheLock.cpp b/clang/lib/Serialization/ModuleCacheLock.cpp
new file mode 100644
index 0000000000000..0c59ed7f28b98
--- /dev/null
+++ b/clang/lib/Serialization/ModuleCacheLock.cpp
@@ -0,0 +1,60 @@
+#include "clang/Serialization/ModuleCacheLock.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang;
+
+namespace {
+struct ModuleCacheFileLockManager : ModuleCacheLockManager {
+  llvm::LockFileManager Lock;
+
+  ModuleCacheFileLockManager(StringRef ModuleFilename) : Lock(ModuleFilename) {}
+
+  operator LockResult() const override {
+    switch (Lock) {
+    case llvm::LockFileManager::LFS_Owned:
+      return LockResult::Owned;
+    case llvm::LockFileManager::LFS_Shared:
+      return LockResult::Shared;
+    case llvm::LockFileManager::LFS_Error:
+      return LockResult::Error;
+    }
+  }
+
+  WaitForUnlockResult waitForUnlock() override {
+    switch (Lock.waitForUnlock()) {
+    case llvm::LockFileManager::Res_Success:
+      return WaitForUnlockResult::Success;
+    case llvm::LockFileManager::Res_OwnerDied:
+      return WaitForUnlockResult::OwnerDied;
+    case llvm::LockFileManager::Res_Timeout:
+      return WaitForUnlockResult::Timeout;
+    }
+  }
+
+  void unsafeRemoveLock() override { Lock.unsafeRemoveLockFile(); }
+
+  std::string getErrorMessage() const override {
+    return Lock.getErrorMessage();
+  }
+};
+
+struct ModuleCacheFileLock : ModuleCacheLock {
+  void prepareLock(StringRef ModuleFilename) override {
+    // FIXME: have LockFileManager return an error_code so that we can
+    // avoid the mkdir when the directory already exists.
+    StringRef Dir = llvm::sys::path::parent_path(ModuleFilename);
+    llvm::sys::fs::create_directories(Dir);
+  }
+
+  std::unique_ptr<ModuleCacheLockManager>
+  tryLock(StringRef ModuleFilename) override {
+    return std::make_unique<ModuleCacheFileLockManager>(ModuleFilename);
+  }
+};
+} // namespace
+
+std::shared_ptr<ModuleCacheLock> clang::getModuleCacheFileLock() {
+  return std::make_unique<ModuleCacheFileLock>();
+}
diff --git a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
index 66795b0be0baa..7ae35e69d7000 100644
--- a/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
+++ b/clang/lib/Tooling/DependencyScanning/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangDependencyScanning
   DependencyScanningService.cpp
   DependencyScanningWorker.cpp
   DependencyScanningTool.cpp
+  ModuleCacheMutexLock.cpp
   ModuleDepCollector.cpp
 
   DEPENDS
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 697f26ee5d12f..fdc86d1764eea 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -22,6 +22,7 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ObjectFilePCHContainerReader.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 #include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -315,7 +316,10 @@ class DependencyScanningAction : public tooling::ToolAction {
     Scanned = true;
 
     // Create a compiler instance to handle the actual work.
-    ScanInstanceStorage.emplace(std::move(PCHContainerOps));
+    ModuleCacheLck =
+        getModuleCacheMutexLock(Service.getSharedModuleCacheMutexes());
+    ScanInstanceStorage.emplace(std::move(PCHContainerOps), nullptr,
+                                ModuleCacheLck);
     CompilerInstance &ScanInstance = *ScanInstanceStorage;
     ScanInstance.setInvocation(std::move(Invocation));
 
@@ -479,6 +483,7 @@ class DependencyScanningAction : public tooling::ToolAction {
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   bool DisableFree;
   std::optional<StringRef> ModuleName;
+  std::shared_ptr<ModuleCacheLock> ModuleCacheLck;
   std::optional<CompilerInstance> ScanInstanceStorage;
   std::shared_ptr<ModuleDepCollector> MDC;
   std::vector<std::string> LastCC1Arguments;
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
new file mode 100644
index 0000000000000..162b5a4b213f5
--- /dev/null
+++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
@@ -0,0 +1,81 @@
+#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace dependencies;
+
+namespace {
+struct ModuleCacheMutexLockManager : ModuleCacheLockManager {
+  ModuleCacheMutexes &Mutexes;
+  StringRef Filename;
+
+  std::shared_ptr<ModuleCacheMutexWrapper> MutexWrapper;
+  bool Owning;
+
+  ModuleCacheMutexLockManager(ModuleCacheMutexes &Mutexes, StringRef Filename)
+      : Mutexes(Mutexes), Filename(Filename) {
+    Owning = false;
+    {
+      std::lock_guard Lock(Mutexes.Mutex);
+      auto &MutexWrapperInMap = Mutexes.Map[Filename];
+      if (!MutexWrapperInMap) {
+        MutexWrapperInMap = std::make_shared<ModuleCacheMutexWrapper>();
+        Owning = true;
+      }
+      // Increment the reference count of the mutex here in the critical section
+      // to guarantee that it's kept alive even when another thread removes it
+      // via \c unsafeRemoveLock().
+      MutexWrapper = MutexWrapperInMap;
+    }
+
+    if (Owning)
+      MutexWrapper->Mutex.lock();
+  }
+
+  ~ModuleCacheMutexLockManager() override {
+    if (Owning) {
+      MutexWrapper->Done = true;
+      MutexWrapper->CondVar.notify_all();
+      MutexWrapper->Mutex.unlock();
+    }
+  }
+
+  operator LockResult() const override {
+    return Owning ? LockResult::Owned : LockResult::Shared;
+  }
+
+  WaitForUnlockResult waitForUnlock() override {
+    assert(!Owning);
+    std::unique_lock Lock(MutexWrapper->Mutex);
+    bool Done = MutexWrapper->CondVar.wait_for(
+        Lock, std::chrono::seconds(90), [&] { return MutexWrapper->Done; });
+    return Done ? WaitForUnlockResult::Success : WaitForUnlockResult::Timeout;
+  }
+
+  void unsafeRemoveLock() override {
+    std::lock_guard Lock(Mutexes.Mutex);
+    Mutexes.Map[Filename].reset();
+  }
+
+  std::string getErrorMessage() const override {
+    llvm_unreachable("ModuleCacheMutexLockManager cannot fail");
+  }
+};
+
+struct ModuleCacheMutexLock : ModuleCacheLock {
+  ModuleCacheMutexes &Mutexes;
+
+  ModuleCacheMutexLock(ModuleCacheMutexes &Mutexes) : Mutexes(Mutexes) {}
+
+  void prepareLock(StringRef Filename) override {}
+
+  std::unique_ptr<ModuleCacheLockManager> tryLock(StringRef Filename) override {
+    return std::make_unique<ModuleCacheMutexLockManager>(Mutexes, Filename);
+  }
+};
+} // namespace
+
+std::shared_ptr<ModuleCacheLock>
+dependencies::getModuleCacheMutexLock(ModuleCacheMutexes &Mutexes) {
+  return std::make_shared<ModuleCacheMutexLock>(Mutexes);
+}

>From 1ac1f238e144ca4677af80c30073ffc56daebca7 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 4 Mar 2025 09:47:46 -0800
Subject: [PATCH 2/5] clang-format

---
 .../Tooling/DependencyScanning/DependencyScanningService.h      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index f6914b90ac67e..d4e4dba8058f1 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -9,8 +9,8 @@
 #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_DEPENDENCYSCANNINGSERVICE_H
 
-#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
+#include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 #include "llvm/ADT/BitmaskEnum.h"
 
 namespace clang {

>From 3e46954d2984083273f8c694fa86c37d7b289711 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 5 Mar 2025 14:27:50 -0800
Subject: [PATCH 3/5] Use `std::shared_mutex`, remove timeouts, remove support
 for `unsafeRemoveLock()`

---
 .../DependencyScanning/ModuleCacheMutexLock.h | 12 +---
 .../ModuleCacheMutexLock.cpp                  | 57 ++++++-------------
 2 files changed, 18 insertions(+), 51 deletions(-)

diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
index 312972b26702c..0e7f9031f153b 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
@@ -4,22 +4,14 @@
 #include "clang/Serialization/ModuleCacheLock.h"
 #include "llvm/ADT/StringMap.h"
 
-#include <condition_variable>
+#include <shared_mutex>
 
 namespace clang {
 namespace tooling {
 namespace dependencies {
-struct ModuleCacheMutexWrapper {
-  std::mutex Mutex;
-  std::condition_variable CondVar;
-  bool Done = false;
-
-  ModuleCacheMutexWrapper() = default;
-};
-
 struct ModuleCacheMutexes {
   std::mutex Mutex;
-  llvm::StringMap<std::shared_ptr<ModuleCacheMutexWrapper>> Map;
+  llvm::StringMap<std::unique_ptr<std::shared_mutex>> Map;
 };
 
 std::shared_ptr<ModuleCacheLock>
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
index 162b5a4b213f5..a6d9526bc13e7 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
@@ -6,55 +6,23 @@ using namespace dependencies;
 
 namespace {
 struct ModuleCacheMutexLockManager : ModuleCacheLockManager {
-  ModuleCacheMutexes &Mutexes;
-  StringRef Filename;
-
-  std::shared_ptr<ModuleCacheMutexWrapper> MutexWrapper;
-  bool Owning;
-
-  ModuleCacheMutexLockManager(ModuleCacheMutexes &Mutexes, StringRef Filename)
-      : Mutexes(Mutexes), Filename(Filename) {
-    Owning = false;
-    {
-      std::lock_guard Lock(Mutexes.Mutex);
-      auto &MutexWrapperInMap = Mutexes.Map[Filename];
-      if (!MutexWrapperInMap) {
-        MutexWrapperInMap = std::make_shared<ModuleCacheMutexWrapper>();
-        Owning = true;
-      }
-      // Increment the reference count of the mutex here in the critical section
-      // to guarantee that it's kept alive even when another thread removes it
-      // via \c unsafeRemoveLock().
-      MutexWrapper = MutexWrapperInMap;
-    }
+  std::unique_lock<std::shared_mutex> OwningLock;
 
-    if (Owning)
-      MutexWrapper->Mutex.lock();
-  }
-
-  ~ModuleCacheMutexLockManager() override {
-    if (Owning) {
-      MutexWrapper->Done = true;
-      MutexWrapper->CondVar.notify_all();
-      MutexWrapper->Mutex.unlock();
-    }
-  }
+  ModuleCacheMutexLockManager(std::shared_mutex &Mutex)
+      : OwningLock(Mutex, std::try_to_lock_t{}) {}
 
   operator LockResult() const override {
-    return Owning ? LockResult::Owned : LockResult::Shared;
+    return OwningLock ? LockResult::Owned : LockResult::Shared;
   }
 
   WaitForUnlockResult waitForUnlock() override {
-    assert(!Owning);
-    std::unique_lock Lock(MutexWrapper->Mutex);
-    bool Done = MutexWrapper->CondVar.wait_for(
-        Lock, std::chrono::seconds(90), [&] { return MutexWrapper->Done; });
-    return Done ? WaitForUnlockResult::Success : WaitForUnlockResult::Timeout;
+    assert(!OwningLock);
+    std::shared_lock Lock(*OwningLock.mutex());
+    return WaitForUnlockResult::Success;
   }
 
   void unsafeRemoveLock() override {
-    std::lock_guard Lock(Mutexes.Mutex);
-    Mutexes.Map[Filename].reset();
+    llvm_unreachable("ModuleCacheMutexLockManager cannot remove locks");
   }
 
   std::string getErrorMessage() const override {
@@ -70,7 +38,14 @@ struct ModuleCacheMutexLock : ModuleCacheLock {
   void prepareLock(StringRef Filename) override {}
 
   std::unique_ptr<ModuleCacheLockManager> tryLock(StringRef Filename) override {
-    return std::make_unique<ModuleCacheMutexLockManager>(Mutexes, Filename);
+    auto &Mutex = [&]() -> std::shared_mutex & {
+      std::lock_guard Lock(Mutexes.Mutex);
+      auto &Mutex = Mutexes.Map[Filename];
+      if (!Mutex)
+        Mutex = std::make_unique<std::shared_mutex>();
+      return *Mutex;
+    }();
+    return std::make_unique<ModuleCacheMutexLockManager>(Mutex);
   }
 };
 } // namespace

>From 60ca622aa6906cbe8f8ebe28d06ae928f3d8f2f6 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 5 Mar 2025 16:54:55 -0800
Subject: [PATCH 4/5] Add missing include

---
 clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
index a6d9526bc13e7..b9a0b8d4d9472 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
@@ -1,5 +1,7 @@
 #include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 
+#include <mutex>
+
 using namespace clang;
 using namespace tooling;
 using namespace dependencies;

>From b55b649f5e88b0bc8bd9a452ab5662481ac59963 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Wed, 5 Mar 2025 16:56:42 -0800
Subject: [PATCH 5/5] Add license header to new files

---
 clang/include/clang/Serialization/ModuleCacheLock.h       | 8 ++++++++
 .../Tooling/DependencyScanning/ModuleCacheMutexLock.h     | 8 ++++++++
 .../Tooling/DependencyScanning/ModuleCacheMutexLock.cpp   | 8 ++++++++
 3 files changed, 24 insertions(+)

diff --git a/clang/include/clang/Serialization/ModuleCacheLock.h b/clang/include/clang/Serialization/ModuleCacheLock.h
index 9435735b1a1bd..414063bcf5f67 100644
--- a/clang/include/clang/Serialization/ModuleCacheLock.h
+++ b/clang/include/clang/Serialization/ModuleCacheLock.h
@@ -1,3 +1,11 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
 #ifndef LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H
 #define LLVM_CLANG_SERIALIZATION_MODULECACHELOCK_H
 
diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
index 0e7f9031f153b..719921b259caa 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h
@@ -1,3 +1,11 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
 #ifndef LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H
 #define LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULECACHEMUTEXLOCK_H
 
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
index b9a0b8d4d9472..95667edd0dcd6 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleCacheMutexLock.cpp
@@ -1,3 +1,11 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
 #include "clang/Tooling/DependencyScanning/ModuleCacheMutexLock.h"
 
 #include <mutex>



More information about the cfe-commits mailing list