diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -146,7 +146,7 @@ // FIXME(ioeric): this can be slow and we may be able to index on less // critical paths. WorkScheduler( - CDB, TUScheduler::Options(Opts), + FSProvider, CDB, TUScheduler::Options(Opts), std::make_unique( DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) { // Adds an index to the stack, at higher priority than existing indexes. @@ -190,7 +190,7 @@ // Compile command is set asynchronously during update, as it can be slow. ParseInputs Inputs; - Inputs.FS = FS; + Inputs.FS = std::move(FS); Inputs.Contents = std::string(Contents); Inputs.Version = Version.str(); Inputs.ForceRebuild = ForceRebuild; diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -13,6 +13,7 @@ #include "Diagnostics.h" #include "GlobalCompilationDatabase.h" #include "index/CanonicalIncludes.h" +#include "support/FSProvider.h" #include "support/Function.h" #include "support/Path.h" #include "support/Threading.h" @@ -197,7 +198,8 @@ bool AsyncPreambleBuilds = false; }; - TUScheduler(const GlobalCompilationDatabase &CDB, const Options &Opts, + TUScheduler(const FileSystemProvider &FSProvider, + const GlobalCompilationDatabase &CDB, const Options &Opts, std::unique_ptr ASTCallbacks = nullptr); ~TUScheduler(); @@ -304,6 +306,7 @@ static llvm::Optional getFileBeingProcessedInContext(); private: + const FileSystemProvider &FSProvider; const GlobalCompilationDatabase &CDB; const Options Opts; std::unique_ptr Callbacks; // not nullptr diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -55,6 +55,7 @@ #include "index/CanonicalIncludes.h" #include "support/Cancellation.h" #include "support/Context.h" +#include "support/FSProvider.h" #include "support/Logger.h" #include "support/Path.h" #include "support/Threading.h" @@ -361,7 +362,8 @@ /// worker. class ASTWorker { friend class ASTWorkerHandle; - ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, + ASTWorker(PathRef FileName, const FileSystemProvider &FSProvider, + const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync, const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); @@ -371,12 +373,11 @@ /// is null, all requests will be processed on the calling thread /// synchronously instead. \p Barrier is acquired when processing each /// request, it is used to limit the number of actively running threads. - static ASTWorkerHandle create(PathRef FileName, - const GlobalCompilationDatabase &CDB, - TUScheduler::ASTCache &IdleASTs, - AsyncTaskRunner *Tasks, Semaphore &Barrier, - const TUScheduler::Options &Opts, - ParsingCallbacks &Callbacks); + static ASTWorkerHandle + create(PathRef FileName, const FileSystemProvider &FSProvider, + const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &IdleASTs, + AsyncTaskRunner *Tasks, Semaphore &Barrier, + const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); ~ASTWorker(); void update(ParseInputs Inputs, WantDiagnostics); @@ -455,6 +456,7 @@ const DebouncePolicy UpdateDebounce; /// File that ASTWorker is responsible for. const Path FileName; + const FileSystemProvider &FSProvider; const GlobalCompilationDatabase &CDB; /// Callback invoked when preamble or main file AST is built. ParsingCallbacks &Callbacks; @@ -541,13 +543,15 @@ }; ASTWorkerHandle ASTWorker::create(PathRef FileName, + const FileSystemProvider &FSProvider, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, Semaphore &Barrier, const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) { - std::shared_ptr Worker(new ASTWorker( - FileName, CDB, IdleASTs, Barrier, /*RunSync=*/!Tasks, Opts, Callbacks)); + std::shared_ptr Worker( + new ASTWorker(FileName, FSProvider, CDB, IdleASTs, Barrier, + /*RunSync=*/!Tasks, Opts, Callbacks)); if (Tasks) { Tasks->runAsync("ASTWorker:" + llvm::sys::path::filename(FileName), [Worker]() { Worker->run(); }); @@ -558,13 +562,15 @@ return ASTWorkerHandle(std::move(Worker)); } -ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, +ASTWorker::ASTWorker(PathRef FileName, const FileSystemProvider &FSProvider, + const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync, const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(Opts.UpdateDebounce), - FileName(FileName), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), - Done(false), Status(FileName, Callbacks), + FileName(FileName), FSProvider(FSProvider), CDB(CDB), + Callbacks(Callbacks), Barrier(Barrier), Done(false), + Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync || !Opts.AsyncPreambleBuilds, Status, *this) { // Set a fallback command because compile command can be accessed before @@ -649,6 +655,9 @@ return; } + // We need a new FS to be used by preamble thread, as the old one can be + // used by astworker thread and FS is not thread-safe. + Inputs.FS = FSProvider.getFileSystem(); PreamblePeer.update(std::move(Invocation), std::move(Inputs), std::move(CompilerInvocationDiags), WantDiags); // Block until first preamble is ready, as patching an empty preamble would @@ -1201,10 +1210,11 @@ ASTWorkerHandle Worker; }; -TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB, +TUScheduler::TUScheduler(const FileSystemProvider &FSProvider, + const GlobalCompilationDatabase &CDB, const Options &Opts, std::unique_ptr Callbacks) - : CDB(CDB), Opts(Opts), + : FSProvider(FSProvider), CDB(CDB), Opts(Opts), Callbacks(Callbacks ? move(Callbacks) : std::make_unique()), Barrier(Opts.AsyncThreadsCount), @@ -1244,7 +1254,7 @@ if (!FD) { // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = - ASTWorker::create(File, CDB, *IdleASTs, + ASTWorker::create(File, FSProvider, CDB, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier, Opts, *Callbacks); FD = std::unique_ptr( diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -73,7 +73,7 @@ ParseInputs getInputs(PathRef File, std::string Contents) { ParseInputs Inputs; Inputs.CompileCommand = *CDB.getCompileCommand(File); - Inputs.FS = buildTestFS(Files, Timestamps); + Inputs.FS = FSProvider.getFileSystem(); Inputs.Contents = std::move(Contents); Inputs.Opts = ParseOptions(); return Inputs; @@ -149,8 +149,7 @@ std::move(CB)); } - llvm::StringMap Files; - llvm::StringMap Timestamps; + MockFSProvider FSProvider; MockCompilationDatabase CDB; }; @@ -158,13 +157,13 @@ TUSchedulerTests::DiagsCallbackKey; TEST_F(TUSchedulerTests, MissingFiles) { - TUScheduler S(CDB, optsForTest()); + TUScheduler S(FSProvider, CDB, optsForTest()); auto Added = testPath("added.cpp"); - Files[Added] = "x"; + FSProvider.Files[Added] = "x"; auto Missing = testPath("missing.cpp"); - Files[Missing] = ""; + FSProvider.Files[Missing] = ""; S.update(Added, getInputs(Added, "x"), WantDiagnostics::No); @@ -205,7 +204,7 @@ // To avoid a racy test, don't allow tasks to actually run on the worker // thread until we've scheduled them all. Notification Ready; - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); auto Path = testPath("foo.cpp"); updateWithDiags(S, Path, "", WantDiagnostics::Yes, [&](std::vector) { Ready.wait(); }); @@ -234,7 +233,7 @@ { auto Opts = optsForTest(); Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1)); - TUScheduler S(CDB, Opts, captureDiags()); + TUScheduler S(FSProvider, CDB, Opts, captureDiags()); // FIXME: we could probably use timeouts lower than 1 second here. auto Path = testPath("foo.cpp"); updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto, @@ -267,7 +266,7 @@ std::vector DiagsSeen, ReadsSeen, ReadsCanceled; { Notification Proceed; // Ensure we schedule everything. - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); auto Path = testPath("foo.cpp"); // Helper to schedule a named update and return a function to cancel it. auto Update = [&](std::string ID) -> Canceler { @@ -324,7 +323,7 @@ TEST_F(TUSchedulerTests, InvalidationNoCrash) { auto Path = testPath("foo.cpp"); - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); Notification StartedRunning; Notification ScheduledChange; @@ -348,7 +347,7 @@ TEST_F(TUSchedulerTests, Invalidation) { auto Path = testPath("foo.cpp"); - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); std::atomic Builds(0), Actions(0); Notification Start; @@ -419,13 +418,13 @@ { auto Opts = optsForTest(); Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(50)); - TUScheduler S(CDB, Opts, captureDiags()); + TUScheduler S(FSProvider, CDB, Opts, captureDiags()); std::vector Files; for (int I = 0; I < FilesCount; ++I) { std::string Name = "foo" + std::to_string(I) + ".cpp"; Files.push_back(testPath(Name)); - this->Files[Files.back()] = ""; + FSProvider.Files[Files.back()] = ""; } StringRef Contents1 = R"cpp(int a;)cpp"; @@ -526,7 +525,7 @@ Opts.AsyncThreadsCount = 1; Opts.RetentionPolicy.MaxRetainedASTs = 2; trace::TestTracer Tracer; - TUScheduler S(CDB, Opts); + TUScheduler S(FSProvider, CDB, Opts); llvm::StringLiteral SourceContents = R"cpp( int* a; @@ -586,7 +585,7 @@ TEST_F(TUSchedulerTests, NoopChangesDontThrashCache) { auto Opts = optsForTest(); Opts.RetentionPolicy.MaxRetainedASTs = 1; - TUScheduler S(CDB, Opts); + TUScheduler S(FSProvider, CDB, Opts); auto Foo = testPath("foo.cpp"); auto FooInputs = getInputs(Foo, "int x=1;"); @@ -612,13 +611,13 @@ } TEST_F(TUSchedulerTests, EmptyPreamble) { - TUScheduler S(CDB, optsForTest()); + TUScheduler S(FSProvider, CDB, optsForTest()); auto Foo = testPath("foo.cpp"); auto Header = testPath("foo.h"); - Files[Header] = "void foo()"; - Timestamps[Header] = time_t(0); + FSProvider.Files[Header] = "void foo()"; + FSProvider.Timestamps[Header] = time_t(0); auto WithPreamble = R"cpp( #include "foo.h" int main() {} @@ -653,7 +652,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) { // Testing strategy: we update the file and schedule a few preamble reads at // the same time. All reads should get the same non-null preamble. - TUScheduler S(CDB, optsForTest()); + TUScheduler S(FSProvider, CDB, optsForTest()); auto Foo = testPath("foo.cpp"); auto NonEmptyPreamble = R"cpp( #define FOO 1 @@ -681,13 +680,13 @@ } TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); auto Source = testPath("foo.cpp"); auto Header = testPath("foo.h"); - Files[Header] = "int a;"; - Timestamps[Header] = time_t(0); + FSProvider.Files[Header] = "int a;"; + FSProvider.Timestamps[Header] = time_t(0); std::string SourceContents = R"cpp( #include "foo.h" @@ -715,7 +714,7 @@ ASSERT_EQ(S.fileStats().lookup(Source).PreambleBuilds, 1u); // Update to a header should cause a rebuild, though. - Timestamps[Header] = time_t(1); + FSProvider.Timestamps[Header] = time_t(1); ASSERT_TRUE(DoUpdate(SourceContents)); ASSERT_FALSE(DoUpdate(SourceContents)); ASSERT_EQ(S.fileStats().lookup(Source).ASTBuilds, 2u); @@ -737,7 +736,7 @@ } TEST_F(TUSchedulerTests, ForceRebuild) { - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); auto Source = testPath("foo.cpp"); auto Header = testPath("foo.h"); @@ -761,11 +760,12 @@ Field(&Diag::Message, "use of undeclared identifier 'a'"))); }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); // Add the header file. We need to recreate the inputs since we changed a // file from underneath the test FS. - Files[Header] = "int a;"; - Timestamps[Header] = time_t(1); + FSProvider.Files[Header] = "int a;"; + FSProvider.Timestamps[Header] = time_t(1); Inputs = getInputs(Source, SourceContents); // The addition of the missing header file shouldn't trigger a rebuild since @@ -776,6 +776,7 @@ ++DiagCount; ADD_FAILURE() << "Did not expect diagnostics for missing header update"; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); // Forcing the reload should should cause a rebuild which no longer has any // errors. @@ -791,7 +792,7 @@ } TEST_F(TUSchedulerTests, NoChangeDiags) { trace::TestTracer Tracer; - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); auto FooCpp = testPath("foo.cpp"); const auto *Contents = "int a; int b;"; @@ -830,7 +831,7 @@ } TEST_F(TUSchedulerTests, Run) { - TUScheduler S(CDB, optsForTest()); + TUScheduler S(FSProvider, CDB, optsForTest()); std::atomic Counter(0); S.run("add 1", [&] { ++Counter; }); S.run("add 2", [&] { Counter += 2; }); @@ -931,7 +932,7 @@ // (!) 'Ready' must live longer than TUScheduler. Notification Ready; - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); std::vector Diagnostics; updateWithDiags(S, testPath("foo.cpp"), "void test() {}", WantDiagnostics::Yes, [&](std::vector D) { @@ -955,7 +956,7 @@ // (!) 'Ready' must live longer than TUScheduler. Notification Ready; - TUScheduler S(CDB, optsForTest(), captureDiags()); + TUScheduler S(FSProvider, CDB, optsForTest(), captureDiags()); std::vector Diagnostics; updateWithDiags(S, testPath("foo.cpp"), "void test() {}", WantDiagnostics::Yes, [&](std::vector D) { @@ -1015,7 +1016,7 @@ static constexpr llvm::StringLiteral InputsV0 = "v0"; static constexpr llvm::StringLiteral InputsV1 = "v1"; Notification Ready; - TUScheduler S(CDB, optsForTest(), + TUScheduler S(FSProvider, CDB, optsForTest(), std::make_unique(InputsV1, Ready)); Path File = testPath("foo.cpp"); diff --git a/clang-tools-extra/clangd/unittests/TestFS.h b/clang-tools-extra/clangd/unittests/TestFS.h --- a/clang-tools-extra/clangd/unittests/TestFS.h +++ b/clang-tools-extra/clangd/unittests/TestFS.h @@ -31,11 +31,12 @@ class MockFSProvider : public FileSystemProvider { public: IntrusiveRefCntPtr getFileSystem() const override { - return buildTestFS(Files); + return buildTestFS(Files, Timestamps); } // If relative paths are used, they are resolved with testPath(). llvm::StringMap Files; + llvm::StringMap Timestamps; }; // A Compilation database that returns a fixed set of compile flags.