https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/181772
This PR fixes a race condition discovered by thread sanitizer in the asynchronous dependency scanner implementaion. The implementation assumed that whenever a new thread is spawned to compile a module, the primary scanning thread must wait for it to finish to read the PCM it produces. This is not true - it's possible for the implicit build on the primary thread to decide to compile the same module too, leaving the asynchronous thread running without any kind of synchronization. This means the TU scan may return, the service may get destroyed, but the asynchronous thread continues running with the VFS caches and module cache implementation destroyed, leading to crashes. This PR fixes this by awaiting all asynchronous threads at the end of a TU scan. >From 7aa5806853848cea8022cff02d443c00ac54fa6a Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 16 Feb 2026 15:55:35 -0800 Subject: [PATCH] [clang][deps] Ensure the service outlives async module compiles --- .../DependencyScannerImpl.cpp | 73 ++++++++++++++----- 1 file changed, 54 insertions(+), 19 deletions(-) diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 28fa2571a24dc..ef2712ac3b56e 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -554,11 +554,42 @@ dependencies::initializeScanInstanceDependencyCollector( return MDC; } +/// Manages (and terminates) the asynchronous compilation of modules. +class AsyncModuleCompiles { + std::mutex Mutex; + bool Stop = false; + // FIXME: Have the service own a thread pool and use that instead. + std::vector<std::thread> Compiles; + +public: + /// Registers the module compilation, unless this instance is about to be + /// destroyed. + void add(llvm::unique_function<void()> Compile) { + std::lock_guard Lock(Mutex); + if (!Stop) + Compiles.emplace_back(std::move(Compile)); + } + + ~AsyncModuleCompiles() { + { + // Prevent registration of further module compiles. + std::lock_guard Lock(Mutex); + Stop = true; + } + + // Wait for outstanding module compiles to finish. + for (std::thread &Compile : Compiles) + Compile.join(); + } +}; + struct SingleModuleWithAsyncModuleCompiles : PreprocessOnlyAction { DependencyScanningService &Service; + AsyncModuleCompiles &Compiles; - SingleModuleWithAsyncModuleCompiles(DependencyScanningService &Service) - : Service(Service) {} + SingleModuleWithAsyncModuleCompiles(DependencyScanningService &Service, + AsyncModuleCompiles &Compiles) + : Service(Service), Compiles(Compiles) {} bool BeginSourceFileAction(CompilerInstance &CI) override; }; @@ -568,9 +599,11 @@ struct SingleModuleWithAsyncModuleCompiles : PreprocessOnlyAction { struct AsyncModuleCompile : PPCallbacks { CompilerInstance &CI; DependencyScanningService &Service; + AsyncModuleCompiles &Compiles; - AsyncModuleCompile(CompilerInstance &CI, DependencyScanningService &Service) - : CI(CI), Service(Service) {} + AsyncModuleCompile(CompilerInstance &CI, DependencyScanningService &Service, + AsyncModuleCompiles &Compiles) + : CI(CI), Service(Service), Compiles(Compiles) {} void moduleLoadSkipped(Module *M) override { M = M->getTopLevelModule(); @@ -635,25 +668,23 @@ struct AsyncModuleCompile : PPCallbacks { auto ModCI2 = CI.cloneForModuleCompile(SourceLocation(), M, ModuleFileName, CloneConfig); - // FIXME: Have the service own a thread pool and use that instead. - // FIXME: This lock belongs to a module cache that might not outlive the - // thread. (This should work for now, because the in-process lock only - // refers to an object managed by the service, which should outlive this - // thread.) - std::thread([Lock = std::move(Lock), ModCI1 = std::move(ModCI1), - ModCI2 = std::move(ModCI2), DC = std::move(DC), - Service = &Service] { + // Note: This lock belongs to a module cache that might not outlive the + // thread. This works, because the in-process lock only refers to an object + // managed by the service, which does outlive the thread. + Compiles.add([Lock = std::move(Lock), ModCI1 = std::move(ModCI1), + ModCI2 = std::move(ModCI2), DC = std::move(DC), + Service = &Service, Compiles = &Compiles] { llvm::CrashRecoveryContext CRC; (void)CRC.RunSafely([&] { // Quickly discovers and compiles modules for the real scan below. - SingleModuleWithAsyncModuleCompiles Action1(*Service); + SingleModuleWithAsyncModuleCompiles Action1(*Service, *Compiles); (void)ModCI1->ExecuteAction(Action1); // The real scan below. ModCI2->getPreprocessorOpts().SingleModuleParseMode = false; GenerateModuleFromModuleMapAction Action2; (void)ModCI2->ExecuteAction(Action2); }); - }).detach(); + }); } }; @@ -661,14 +692,16 @@ struct AsyncModuleCompile : PPCallbacks { /// modules asynchronously without blocking or importing them. struct SingleTUWithAsyncModuleCompiles : PreprocessOnlyAction { DependencyScanningService &Service; + AsyncModuleCompiles &Compiles; - SingleTUWithAsyncModuleCompiles(DependencyScanningService &Service) - : Service(Service) {} + SingleTUWithAsyncModuleCompiles(DependencyScanningService &Service, + AsyncModuleCompiles &Compiles) + : Service(Service), Compiles(Compiles) {} bool BeginSourceFileAction(CompilerInstance &CI) override { CI.getInvocation().getPreprocessorOpts().SingleModuleParseMode = true; CI.getPreprocessor().addPPCallbacks( - std::make_unique<AsyncModuleCompile>(CI, Service)); + std::make_unique<AsyncModuleCompile>(CI, Service, Compiles)); return true; } }; @@ -677,7 +710,7 @@ bool SingleModuleWithAsyncModuleCompiles::BeginSourceFileAction( CompilerInstance &CI) { CI.getInvocation().getPreprocessorOpts().SingleModuleParseMode = true; CI.getPreprocessor().addPPCallbacks( - std::make_unique<AsyncModuleCompile>(CI, Service)); + std::make_unique<AsyncModuleCompile>(CI, Service, Compiles)); return true; } @@ -711,6 +744,7 @@ bool DependencyScanningAction::runInvocation( createScanCompilerInvocation(*OriginalInvocation, Service); // Quickly discovers and compiles modules for the real scan below. + std::optional<AsyncModuleCompiles> AsyncCompiles; if (Service.getOpts().AsyncScanModules) { auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries()); auto ScanInstanceStorage = std::make_unique<CompilerInstance>( @@ -733,7 +767,8 @@ bool DependencyScanningAction::runInvocation( if (ScanInstance.getFrontendOpts().ProgramAction == frontend::GeneratePCH) ScanInstance.getLangOpts().CompilingPCH = true; - SingleTUWithAsyncModuleCompiles Action(Service); + AsyncCompiles.emplace(); + SingleTUWithAsyncModuleCompiles Action(Service, *AsyncCompiles); (void)ScanInstance.ExecuteAction(Action); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
