[PATCH] D140486: [clangd] Fix crashing race in ClangdServer shutdown with stdlib indexing

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rGb494f67f6796: [clangd] Fix crashing race in ClangdServer shutdown with stdlib indexing (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D140486?vs=484612&id=484683#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140486/new/

https://reviews.llvm.org/D140486

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -63,7 +63,7 @@
                        ClangdServer::Callbacks *ServerCallbacks,
                        const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks)
       : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
-        Tasks(Tasks) {}
+        Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks) {}
 
   void onPreambleAST(PathRef Path, llvm::StringRef Version,
                      const CompilerInvocation &CI, ASTContext &Ctx,
@@ -71,7 +71,7 @@
                      const CanonicalIncludes &CanonIncludes) override {
     // If this preamble uses a standard library we haven't seen yet, index it.
     if (FIndex)
-      if (auto Loc = Stdlib.add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
+      if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
         indexStdlib(CI, std::move(*Loc));
 
     if (FIndex)
@@ -79,11 +79,20 @@
   }
 
   void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
-    auto Task = [this, LO(*CI.getLangOpts()), Loc(std::move(Loc)),
-                 CI(std::make_unique<CompilerInvocation>(CI))]() mutable {
+    // This task is owned by Tasks, which outlives the TUScheduler and
+    // therefore the UpdateIndexCallbacks.
+    // We must be careful that the references we capture outlive TUScheduler.
+    auto Task = [LO(*CI.getLangOpts()), Loc(std::move(Loc)),
+                 CI(std::make_unique<CompilerInvocation>(CI)),
+                 // External values that outlive ClangdServer
+                 TFS(&TFS),
+                 // Index outlives TUScheduler (declared first)
+                 FIndex(FIndex),
+                 // shared_ptr extends lifetime
+                 Stdlib(Stdlib)]() mutable {
       IndexFileIn IF;
-      IF.Symbols = indexStandardLibrary(std::move(CI), Loc, TFS);
-      if (Stdlib.isBest(LO))
+      IF.Symbols = indexStandardLibrary(std::move(CI), Loc, *TFS);
+      if (Stdlib->isBest(LO))
         FIndex->updatePreamble(std::move(IF));
     };
     if (Tasks)
@@ -128,7 +137,7 @@
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
   const ThreadsafeFS &TFS;
-  StdLibSet Stdlib;
+  std::shared_ptr<StdLibSet> Stdlib;
   AsyncTaskRunner *Tasks;
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140486.484683.patch
Type: text/x-patch
Size: 2413 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221221/6bdf9d83/attachment.bin>