https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/182722
>From 36b22cba04905a4c4538b3764d57cd35832dd5bc Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 23 Feb 2026 16:33:00 -0800 Subject: [PATCH 1/5] [clang] Introduce `-fimplicit-modules-lock-timeout=<seconds>` --- clang/include/clang/Frontend/FrontendOptions.h | 3 +++ clang/include/clang/Options/Options.td | 6 ++++++ clang/lib/Driver/ToolChains/Clang.cpp | 2 ++ clang/lib/Frontend/CompilerInstance.cpp | 4 +++- 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index ba7da56cb9fce..357b33ed95b2b 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -491,6 +491,9 @@ class FrontendOptions { /// The list of files to embed into the compiled module file. std::vector<std::string> ModulesEmbedFiles; + /// The time in seconds to wait on an implicit module lock before timing out. + unsigned ImplicitModulesLockTimeoutSeconds = 90; + /// The list of AST files to merge. std::vector<std::string> ASTMergeFiles; diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 4ac812e92e2cb..57ee626dfe132 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -3488,6 +3488,12 @@ def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, CC1Option]>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; +def fimplicit_modules_lock_timeout_EQ : Joined<["-"], "fimplicit-modules-lock-timeout=">, + Flags<[NoXarchOption]>, Visibility<[ClangOption, CLOption, CC1Option]>, + MetaVarName<"<seconds>">, + HelpText<"Time to wait on an implicit module lock before timing out">, + MarshallingInfoInt<FrontendOpts<"ImplicitModulesLockTimeoutSeconds">, "90">; + defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", LangOpts<"SkipODRCheckInGMF">, DefaultFalse, PosFlag<SetTrue, [], [CC1Option], diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 6af05130f6431..49984f1504c67 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3911,6 +3911,8 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D, Path.insert(Path.begin(), Arg, Arg + strlen(Arg)); CmdArgs.push_back(Args.MakeArgString(Path)); } + + Args.AddLastArg(CmdArgs, options::OPT_fimplicit_modules_lock_timeout_EQ); } if (HaveModules) { diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 9f1a3c56feec1..1567407e99674 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1514,7 +1514,9 @@ static bool compileModuleAndReadASTBehindLock( // Someone else is responsible for building the module. Wait for them to // finish. - switch (Lock->waitForUnlockFor(std::chrono::seconds(90))) { + unsigned Timeout = + ImportingInstance.getFrontendOpts().ImplicitModulesLockTimeoutSeconds; + switch (Lock->waitForUnlockFor(std::chrono::seconds(Timeout))) { case llvm::WaitForUnlockResult::Success: break; // The interesting case. case llvm::WaitForUnlockResult::OwnerDied: >From 45b6634964f9e0e08b24109ccd77842cd9e8fefa Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 23 Feb 2026 16:33:42 -0800 Subject: [PATCH 2/5] [clang] Introduce `#pragma clang __debug sleep` --- clang/lib/Lex/Pragma.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp index bba3c89bed38f..1a5a6ce61ecf1 100644 --- a/clang/lib/Lex/Pragma.cpp +++ b/clang/lib/Lex/Pragma.cpp @@ -46,6 +46,7 @@ #include <cstdint> #include <optional> #include <string> +#include <thread> #include <utility> #include <vector> @@ -1079,6 +1080,8 @@ struct PragmaDebugHandler : public PragmaHandler { Crasher.setAnnotationRange(SourceRange(Tok.getLocation())); PP.EnterToken(Crasher, /*IsReinject*/ false); } + } else if (II->isStr("sleep")) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); } else if (II->isStr("dump")) { Token DumpAnnot; DumpAnnot.startToken(); >From dffa129f778ece9b2cfa50226c3d07fae4611eed Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 23 Feb 2026 16:40:14 -0800 Subject: [PATCH 3/5] [clang][deps] Implement module cache lock timeout --- .../DependencyScanning/InProcessModuleCache.h | 2 +- .../InProcessModuleCache.cpp | 23 ++++++------ clang/lib/Frontend/CompilerInstance.cpp | 6 ++- .../ClangScanDeps/modules-cycle-deadlock.c | 37 +++++++++++++++++++ clang/test/Modules/implicit-cycle-deadlock.c | 36 ++++++++++++++++++ 5 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 clang/test/ClangScanDeps/modules-cycle-deadlock.c create mode 100644 clang/test/Modules/implicit-cycle-deadlock.c diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h index 4c95171a2e21e..96f74725b1e06 100644 --- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h +++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h @@ -20,7 +20,7 @@ namespace clang { namespace dependencies { struct ModuleCacheEntry { - std::shared_mutex CompilationMutex; + std::shared_timed_mutex CompilationMutex; std::atomic<std::time_t> Timestamp = 0; }; diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index 4d5dd0c43112c..add43813917fd 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -20,10 +20,10 @@ using namespace dependencies; namespace { class ReaderWriterLock : public llvm::AdvisoryLock { // TODO: Consider using std::atomic::{wait,notify_all} when we move to C++20. - std::unique_lock<std::shared_mutex> OwningLock; + std::unique_lock<std::shared_timed_mutex> OwningLock; public: - ReaderWriterLock(std::shared_mutex &Mutex) + ReaderWriterLock(std::shared_timed_mutex &Mutex) : OwningLock(Mutex, std::defer_lock) {} Expected<bool> tryLock() override { return OwningLock.try_lock(); } @@ -31,18 +31,17 @@ class ReaderWriterLock : public llvm::AdvisoryLock { llvm::WaitForUnlockResult waitForUnlockFor(std::chrono::seconds MaxSeconds) override { assert(!OwningLock); - // We do not respect the timeout here. It's very generous for implicit - // modules, so we'd typically only reach it if the owner crashed (but so did - // we, since we run in the same process), or encountered deadlock. - (void)MaxSeconds; - std::shared_lock<std::shared_mutex> Lock(*OwningLock.mutex()); - return llvm::WaitForUnlockResult::Success; + std::shared_lock<std::shared_timed_mutex> Lock(*OwningLock.mutex(), + MaxSeconds); + return Lock ? llvm::WaitForUnlockResult::Success + : llvm::WaitForUnlockResult::Timeout; } std::error_code unsafeMaybeUnlock() override { - // Unlocking the mutex here would trigger UB and we don't expect this to be - // actually called when compiling scanning modules due to the no-timeout - // guarantee above. + // Only the thread that locked a mutex can unlock it without triggering UB. + // We're forced to ignore the request with the understanding that we will + // not unblock other threads that are currently waiting, and they will have + // to time out themselves. return {}; } @@ -64,7 +63,7 @@ class InProcessModuleCache : public ModuleCache { void prepareForGetLock(StringRef Filename) override {} std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override { - auto &CompilationMutex = [&]() -> std::shared_mutex & { + auto &CompilationMutex = [&]() -> std::shared_timed_mutex & { std::lock_guard<std::mutex> Lock(Entries.Mutex); auto &Entry = Entries.Map[Filename]; if (!Entry) diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 1567407e99674..e8ec4c990b045 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1529,7 +1529,11 @@ static bool compileModuleAndReadASTBehindLock( << Module->Name; // Clear the lock file so that future invocations can make progress. Lock->unsafeMaybeUnlock(); - continue; + // Instead of trying to acquire the lock again (which can fail + // indefinitely with lock implementations that do not support unsafe + // unlock), compile the module ourselves. + return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, + ModuleNameLoc, Module, ModuleFileName); } // Read the module that was just written by someone else. diff --git a/clang/test/ClangScanDeps/modules-cycle-deadlock.c b/clang/test/ClangScanDeps/modules-cycle-deadlock.c new file mode 100644 index 0000000000000..2a800ff7f2f20 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-cycle-deadlock.c @@ -0,0 +1,37 @@ +// This test checks that implicit modules do not encounter a deadlock on a dependency cycle. + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json +// RUN: not clang-scan-deps -mode preprocess -format experimental-full \ +// RUN: -compilation-database %t/cdb.json -j 2 -o %t/out 2> %t/err +// RUN: FileCheck %s --input-file %t/err +// CHECK-DAG: fatal error: cyclic dependency in module 'M': M -> N -> M +// CHECK-DAG: fatal error: cyclic dependency in module 'N': N -> M -> N + +//--- cdb.json.in +[ + { + "file": "DIR/tu1.c", + "directory": "DIR", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fimplicit-modules-lock-timeout=3 DIR/tu1.c -o DIR/tu1.o" + }, + { + "file": "DIR/tu2.c", + "directory": "DIR", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fimplicit-modules-lock-timeout=3 DIR/tu2.c -o DIR/tu2.o" + } +] +//--- tu1.c +#include "m.h" +//--- tu2.c +#include "n.h" +//--- module.modulemap +module M { header "m.h" } +module N { header "n.h" } +//--- m.h +#pragma clang __debug sleep // Give enough time for tu2.c to start building N. +#include "n.h" +//--- n.h +#pragma clang __debug sleep // Give enough time for tu1.c to start building M. +#include "m.h" diff --git a/clang/test/Modules/implicit-cycle-deadlock.c b/clang/test/Modules/implicit-cycle-deadlock.c new file mode 100644 index 0000000000000..226c80c9b9fa2 --- /dev/null +++ b/clang/test/Modules/implicit-cycle-deadlock.c @@ -0,0 +1,36 @@ +// This test checks that implicit modules do not encounter a deadlock on a dependency cycle. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %python %t/run_concurrently.py \ +// RUN: "not %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/cache -fimplicit-modules-lock-timeout=3 %t/tu1.c 2> %t/err1" \ +// RUN: "not %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/cache -fimplicit-modules-lock-timeout=3 %t/tu2.c 2> %t/err2" + +// RUN: FileCheck %s --input-file %t/err1 --check-prefix=CHECK1 +// RUN: FileCheck %s --input-file %t/err2 --check-prefix=CHECK2 +// CHECK1: fatal error: cyclic dependency in module 'M': M -> N -> M +// CHECK2: fatal error: cyclic dependency in module 'N': N -> M -> N + +//--- run_concurrently.py +import subprocess, sys, threading + +def run(cmd): + subprocess.run(cmd, shell=True) + +threads = [threading.Thread(target=run, args=(cmd,)) for cmd in sys.argv[1:]] +for t in threads: t.start() +for t in threads: t.join() +//--- tu1.c +#include "m.h" +//--- tu2.c +#include "n.h" +//--- module.modulemap +module M { header "m.h" } +module N { header "n.h" } +//--- m.h +#pragma clang __debug sleep // Give enough time for tu2.c to start building N. +#include "n.h" +//--- n.h +#pragma clang __debug sleep // Give enough time for tu1.c to start building M. +#include "m.h" >From 170acee0333ed9225e3bd8d8f5cbcc7d280b0f67 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Sun, 22 Feb 2026 14:56:00 -0800 Subject: [PATCH 4/5] [clang][deps] Implement module cache unsafe unlock --- .../DependencyScanning/InProcessModuleCache.h | 3 ++- .../InProcessModuleCache.cpp | 24 ++++++++++++------- clang/lib/Frontend/CompilerInstance.cpp | 10 +++----- llvm/include/llvm/Support/AdvisoryLock.h | 2 +- llvm/include/llvm/Support/LockFileManager.h | 2 +- llvm/lib/Support/LockFileManager.cpp | 2 +- llvm/lib/Support/VirtualOutputBackends.cpp | 6 ++--- llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp | 2 +- 8 files changed, 28 insertions(+), 23 deletions(-) diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h index 96f74725b1e06..e2417761b2c1f 100644 --- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h +++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h @@ -20,7 +20,8 @@ namespace clang { namespace dependencies { struct ModuleCacheEntry { - std::shared_timed_mutex CompilationMutex; + std::shared_ptr<std::shared_timed_mutex> CompilationMutex = + std::make_shared<std::shared_timed_mutex>(); std::atomic<std::time_t> Timestamp = 0; }; diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index add43813917fd..6d58822446420 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -19,12 +19,21 @@ using namespace dependencies; namespace { class ReaderWriterLock : public llvm::AdvisoryLock { + /// Reference to the mutex shared by module cache clients. When unsafe unlock + /// is requested, we override it using std::atomic_store(). To obtain it, we + /// use std::atomic_load(). + std::shared_ptr<std::shared_timed_mutex> &SharedMutex; + /// The mutex we co-own with other threads. This is kept alive for the entire + /// lifetime of this class. Unsafe unlock does not affect it. + std::shared_ptr<std::shared_timed_mutex> CoOwnedMutex; + // TODO: Consider using std::atomic::{wait,notify_all} when we move to C++20. std::unique_lock<std::shared_timed_mutex> OwningLock; public: - ReaderWriterLock(std::shared_timed_mutex &Mutex) - : OwningLock(Mutex, std::defer_lock) {} + ReaderWriterLock(std::shared_ptr<std::shared_timed_mutex> &M) + : SharedMutex(M), CoOwnedMutex(std::atomic_load(&SharedMutex)), + OwningLock(*CoOwnedMutex, std::defer_lock) {} Expected<bool> tryLock() override { return OwningLock.try_lock(); } @@ -37,11 +46,9 @@ class ReaderWriterLock : public llvm::AdvisoryLock { : llvm::WaitForUnlockResult::Timeout; } - std::error_code unsafeMaybeUnlock() override { - // Only the thread that locked a mutex can unlock it without triggering UB. - // We're forced to ignore the request with the understanding that we will - // not unblock other threads that are currently waiting, and they will have - // to time out themselves. + std::error_code unsafeUnlock() override { + std::atomic_store(&SharedMutex, + std::make_shared<std::shared_timed_mutex>()); return {}; } @@ -63,7 +70,8 @@ class InProcessModuleCache : public ModuleCache { void prepareForGetLock(StringRef Filename) override {} std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override { - auto &CompilationMutex = [&]() -> std::shared_timed_mutex & { + auto &CompilationMutex = + [&]() -> std::shared_ptr<std::shared_timed_mutex> & { std::lock_guard<std::mutex> Lock(Entries.Mutex); auto &Entry = Entries.Map[Filename]; if (!Entry) diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index e8ec4c990b045..1e256a9d5614c 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1524,16 +1524,12 @@ static bool compileModuleAndReadASTBehindLock( case llvm::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. + // twice. In case of timeout, try to build it ourselves again. Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout) << Module->Name; // Clear the lock file so that future invocations can make progress. - Lock->unsafeMaybeUnlock(); - // Instead of trying to acquire the lock again (which can fail - // indefinitely with lock implementations that do not support unsafe - // unlock), compile the module ourselves. - return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc, - ModuleNameLoc, Module, ModuleFileName); + Lock->unsafeUnlock(); + continue; } // Read the module that was just written by someone else. diff --git a/llvm/include/llvm/Support/AdvisoryLock.h b/llvm/include/llvm/Support/AdvisoryLock.h index d1c3ccc187e64..03be1fcbe7243 100644 --- a/llvm/include/llvm/Support/AdvisoryLock.h +++ b/llvm/include/llvm/Support/AdvisoryLock.h @@ -48,7 +48,7 @@ class AdvisoryLock { /// For a lock owned by someone else, unlock it. A permitted side-effect is /// that another thread/process may acquire ownership of the lock before the /// existing owner unlocks it. This is an unsafe operation. - virtual std::error_code unsafeMaybeUnlock() = 0; + virtual std::error_code unsafeUnlock() = 0; /// Unlocks the lock if its ownership was previously acquired by \c tryLock(). virtual ~AdvisoryLock() = default; diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h index 1c579ea866f65..69f175bd79f7c 100644 --- a/llvm/include/llvm/Support/LockFileManager.h +++ b/llvm/include/llvm/Support/LockFileManager.h @@ -61,7 +61,7 @@ class LLVM_ABI LockFileManager : public AdvisoryLock { /// Remove the lock file. This may delete a different lock file than /// the one previously read if there is a race. - std::error_code unsafeMaybeUnlock() override; + std::error_code unsafeUnlock() override; /// Unlocks the lock if previously acquired by \c tryLock(). ~LockFileManager() override; diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp index 0891ef980668b..1a98ec03486db 100644 --- a/llvm/lib/Support/LockFileManager.cpp +++ b/llvm/lib/Support/LockFileManager.cpp @@ -294,7 +294,7 @@ LockFileManager::waitForUnlockFor(std::chrono::seconds MaxSeconds) { return WaitForUnlockResult::Timeout; } -std::error_code LockFileManager::unsafeMaybeUnlock() { +std::error_code LockFileManager::unsafeUnlock() { auto BypassSandbox = sys::sandbox::scopedDisable(); return sys::fs::remove(LockFileName); diff --git a/llvm/lib/Support/VirtualOutputBackends.cpp b/llvm/lib/Support/VirtualOutputBackends.cpp index d7a91dfa1471f..0c6ce825d02d0 100644 --- a/llvm/lib/Support/VirtualOutputBackends.cpp +++ b/llvm/lib/Support/VirtualOutputBackends.cpp @@ -496,7 +496,7 @@ Error OnDiskOutputFile::keep() { if (Error Err = Lock.tryLock().moveInto(Owned)) { // If we error acquiring a lock, we cannot ensure appends // to the trace file are atomic - cannot ensure output correctness. - Lock.unsafeMaybeUnlock(); + Lock.unsafeUnlock(); return convertToOutputError( OutputPath, std::make_error_code(std::errc::no_lock_available)); } @@ -508,7 +508,7 @@ Error OnDiskOutputFile::keep() { return convertToOutputError(OutputPath, EC); Out << (*Content)->getBuffer(); Out.close(); - Lock.unsafeMaybeUnlock(); + Lock.unsafeUnlock(); if (Out.has_error()) return convertToOutputError(OutputPath, Out.error()); // Remove temp file and done. @@ -528,7 +528,7 @@ Error OnDiskOutputFile::keep() { // the lock, causing other waiting processes to time-out. Let's clear // the lock and try again right away. If we do start seeing compiler // hangs in this location, we will need to re-consider. - Lock.unsafeMaybeUnlock(); + Lock.unsafeUnlock(); continue; } } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp index d04dc3e25f2dc..c155d85df1769 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp @@ -1575,7 +1575,7 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M, dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug " "output may be mangled by other processes\n"); - Lock.unsafeMaybeUnlock(); + Lock.unsafeUnlock(); break; // give up } } >From 5dc6994d0dd9f42eccb247c2614a09c1b97de842 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 23 Feb 2026 11:06:34 -0800 Subject: [PATCH 5/5] [clang][deps] Implement module cache where unsafe unlock instantly notifies waiting threads --- .../DependencyScanning/InProcessModuleCache.h | 9 ++- .../InProcessModuleCache.cpp | 72 +++++++++++-------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/clang/include/clang/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/DependencyScanning/InProcessModuleCache.h index e2417761b2c1f..8ad9811606611 100644 --- a/clang/include/clang/DependencyScanning/InProcessModuleCache.h +++ b/clang/include/clang/DependencyScanning/InProcessModuleCache.h @@ -13,15 +13,18 @@ #include "llvm/ADT/StringMap.h" #include <atomic> +#include <condition_variable> #include <mutex> -#include <shared_mutex> namespace clang { namespace dependencies { struct ModuleCacheEntry { - std::shared_ptr<std::shared_timed_mutex> CompilationMutex = - std::make_shared<std::shared_timed_mutex>(); + std::mutex Mutex; + std::condition_variable CondVar; + bool Locked = false; + unsigned Generation = 0; + std::atomic<std::time_t> Timestamp = 0; }; diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index 6d58822446420..cd7385c8f38c2 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -12,47 +12,62 @@ #include "llvm/Support/AdvisoryLock.h" #include "llvm/Support/Chrono.h" -#include <mutex> - using namespace clang; using namespace dependencies; namespace { class ReaderWriterLock : public llvm::AdvisoryLock { - /// Reference to the mutex shared by module cache clients. When unsafe unlock - /// is requested, we override it using std::atomic_store(). To obtain it, we - /// use std::atomic_load(). - std::shared_ptr<std::shared_timed_mutex> &SharedMutex; - /// The mutex we co-own with other threads. This is kept alive for the entire - /// lifetime of this class. Unsafe unlock does not affect it. - std::shared_ptr<std::shared_timed_mutex> CoOwnedMutex; - - // TODO: Consider using std::atomic::{wait,notify_all} when we move to C++20. - std::unique_lock<std::shared_timed_mutex> OwningLock; + ModuleCacheEntry &Entry; + std::optional<unsigned> OwnedGeneration; public: - ReaderWriterLock(std::shared_ptr<std::shared_timed_mutex> &M) - : SharedMutex(M), CoOwnedMutex(std::atomic_load(&SharedMutex)), - OwningLock(*CoOwnedMutex, std::defer_lock) {} - - Expected<bool> tryLock() override { return OwningLock.try_lock(); } + ReaderWriterLock(ModuleCacheEntry &Entry) : Entry(Entry) {} + + Expected<bool> tryLock() override { + std::lock_guard<std::mutex> Lock(Entry.Mutex); + if (Entry.Locked) + return false; + Entry.Locked = true; + OwnedGeneration = Entry.Generation; + return true; + } llvm::WaitForUnlockResult waitForUnlockFor(std::chrono::seconds MaxSeconds) override { - assert(!OwningLock); - std::shared_lock<std::shared_timed_mutex> Lock(*OwningLock.mutex(), - MaxSeconds); - return Lock ? llvm::WaitForUnlockResult::Success - : llvm::WaitForUnlockResult::Timeout; + assert(!OwnedGeneration); + std::unique_lock<std::mutex> Lock(Entry.Mutex); + unsigned CurrentGeneration = Entry.Generation; + bool Success = Entry.CondVar.wait_for(Lock, MaxSeconds, [&] { + // We check not only Locked, but also Generation to break the wait in case + // of unsafeUnlock() and successful tryLock(). + return !Entry.Locked || Entry.Generation != CurrentGeneration; + }); + return Success ? llvm::WaitForUnlockResult::Success + : llvm::WaitForUnlockResult::Timeout; } std::error_code unsafeUnlock() override { - std::atomic_store(&SharedMutex, - std::make_shared<std::shared_timed_mutex>()); + { + std::lock_guard<std::mutex> Lock(Entry.Mutex); + Entry.Generation += 1; + Entry.Locked = false; + } + Entry.CondVar.notify_all(); return {}; } - ~ReaderWriterLock() override = default; + ~ReaderWriterLock() override { + if (OwnedGeneration) { + { + std::lock_guard<std::mutex> Lock(Entry.Mutex); + // Avoid stomping over the state managed by someone else after + // unsafeUnlock() and successful tryLock(). + if (*OwnedGeneration == Entry.Generation) + Entry.Locked = false; + } + Entry.CondVar.notify_all(); + } + } }; class InProcessModuleCache : public ModuleCache { @@ -70,15 +85,14 @@ class InProcessModuleCache : public ModuleCache { void prepareForGetLock(StringRef Filename) override {} std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override { - auto &CompilationMutex = - [&]() -> std::shared_ptr<std::shared_timed_mutex> & { + auto &Entry = [&]() -> ModuleCacheEntry & { std::lock_guard<std::mutex> Lock(Entries.Mutex); auto &Entry = Entries.Map[Filename]; if (!Entry) Entry = std::make_unique<ModuleCacheEntry>(); - return Entry->CompilationMutex; + return *Entry; }(); - return std::make_unique<ReaderWriterLock>(CompilationMutex); + return std::make_unique<ReaderWriterLock>(Entry); } std::time_t getModuleTimestamp(StringRef Filename) override { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
