diff --git a/clang-tools-extra/clangd/support/Threading.h b/clang-tools-extra/clangd/support/Threading.h --- a/clang-tools-extra/clangd/support/Threading.h +++ b/clang-tools-extra/clangd/support/Threading.h @@ -24,20 +24,6 @@ namespace clang { namespace clangd { -/// A threadsafe flag that is initially clear. -class Notification { -public: - // Sets the flag. No-op if already set. - void notify(); - // Blocks until flag is set. - void wait() const; - -private: - bool Notified = false; - mutable std::condition_variable CV; - mutable std::mutex Mu; -}; - /// Limits the number of threads that can acquire the lock at the same time. class Semaphore { public: @@ -100,6 +86,21 @@ return true; } +/// A threadsafe flag that is initially clear. +class Notification { +public: + // Sets the flag. No-op if already set. + void notify(); + // Blocks until flag is set. + void wait() const { (void)wait(Deadline::infinity()); } + LLVM_NODISCARD bool wait(Deadline D) const; + +private: + bool Notified = false; + mutable std::condition_variable CV; + mutable std::mutex Mu; +}; + /// Runs tasks on separate (detached) threads and wait for all tasks to finish. /// Objects that need to spawn threads can own an AsyncTaskRunner to ensure they /// all complete on destruction. diff --git a/clang-tools-extra/clangd/support/Threading.cpp b/clang-tools-extra/clangd/support/Threading.cpp --- a/clang-tools-extra/clangd/support/Threading.cpp +++ b/clang-tools-extra/clangd/support/Threading.cpp @@ -34,9 +34,9 @@ } } -void Notification::wait() const { +bool Notification::wait(Deadline D) const { std::unique_lock Lock(Mu); - CV.wait(Lock, [this] { return Notified; }); + return clangd::wait(Lock, CV, D, [&] { return Notified; }); } Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {} 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 @@ -247,28 +247,31 @@ } TEST_F(TUSchedulerTests, Debounce) { - std::atomic CallbackCount(0); - { - auto Opts = optsForTest(); - Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1)); - TUScheduler S(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, - [&](std::vector) { - ADD_FAILURE() - << "auto should have been debounced and canceled"; - }); - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - updateWithDiags(S, Path, "auto (timed out)", WantDiagnostics::Auto, - [&](std::vector) { ++CallbackCount; }); - std::this_thread::sleep_for(std::chrono::seconds(2)); - updateWithDiags(S, Path, "auto (shut down)", WantDiagnostics::Auto, - [&](std::vector) { ++CallbackCount; }); - - ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); - } - EXPECT_EQ(2, CallbackCount); + auto Opts = optsForTest(); + Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(500)); + TUScheduler S(CDB, Opts, captureDiags()); + auto Path = testPath("foo.cpp"); + // Issue a write that's going to be debounced away. + updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto, + [&](std::vector) { + ADD_FAILURE() + << "auto should have been debounced and canceled"; + }); + // Sleep a bit to verify that it's really debounce that's holding diagnostics. + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // Issue another write, this time we'll wait for its diagnostics. + Notification N; + updateWithDiags(S, Path, "auto (timed out)", WantDiagnostics::Auto, + [&](std::vector) { N.notify(); }); + EXPECT_TRUE(N.wait(timeoutSeconds(1))); + + // Once we start shutting down the TUScheduler, this one becomes a dead write. + updateWithDiags(S, Path, "auto (discarded)", WantDiagnostics::Auto, + [&](std::vector) { + ADD_FAILURE() + << "auto should have been discarded (dead write)"; + }); } TEST_F(TUSchedulerTests, Cancellation) {