Index: clangd/Cancellation.h =================================================================== --- clangd/Cancellation.h +++ clangd/Cancellation.h @@ -6,124 +6,82 @@ // License. See LICENSE.TXT for details. // //===----------------------------------------------------------------------===// -// Cancellation mechanism for async tasks. Roughly all the clients of this code -// can be classified into three categories: -// 1. The code that creates and schedules async tasks, e.g. TUScheduler. -// 2. The callers of the async method that can cancel some of the running tasks, -// e.g. `ClangdLSPServer` -// 3. The code running inside the async task itself, i.e. code completion or -// find definition implementation that run clang, etc. -// -// For (1), the guideline is to accept a callback for the result of async -// operation and return a `TaskHandle` to allow cancelling the request. -// -// TaskHandle someAsyncMethod(Runnable T, -// function)> Callback) { -// auto TH = Task::createHandle(); -// WithContext ContextWithCancellationToken(TH); -// auto run = [](){ -// Callback(T()); +// Cancellation mechanism for long-running tasks. +// +// This manages interactions between: +// +// 1. Client code that starts some long-running work, and maybe cancels later. +// +// std::pair Task = cancelableTask(); +// { +// WithContext Cancelable(std::move(Task.first)); +// Expected +// deepThoughtAsync([](int answer){ errs() << answer; }); // } -// // Start run() in a new async thread, and make sure to propagate Context. -// return TH; -// } -// -// The callers of async methods (2) can issue cancellations and should be -// prepared to handle `TaskCancelledError` result: -// -// void Caller() { -// // You should store this handle if you wanna cancel the task later on. -// TaskHandle TH = someAsyncMethod(Task, [](llvm::Expected R) { -// if(/*check for task cancellation error*/) -// // Handle the error -// // Do other things on R. -// }); -// // To cancel the task: -// sleep(5); -// TH->cancel(); -// } -// -// The worker code itself (3) should check for cancellations using -// `Task::isCancelled` that can be retrieved via `getCurrentTask()`. -// -// llvm::Expected AsyncTask() { -// // You can either store the read only TaskHandle by calling getCurrentTask -// // once and just use the variable everytime you want to check for -// // cancellation, or call isCancelled everytime. The former is more -// // efficient if you are going to have multiple checks. -// const auto T = getCurrentTask(); -// // DO SMTHNG... -// if(T.isCancelled()) { -// // Task has been cancelled, lets get out. -// return llvm::makeError(); -// } -// // DO SOME MORE THING... -// if(T.isCancelled()) { -// // Task has been cancelled, lets get out. -// return llvm::makeError(); -// } -// return ResultType(...); -// } -// If the operation was cancelled before task could run to completion, it should -// propagate the TaskCancelledError as a result. +// // ...some time later... +// if (User.fellAsleep()) +// Task.second(); +// +// (This example has an asynchronous computation, but synchronous examples +// work similarly - the Canceler should be invoked from another thread). +// +// 2. Library code that executes long-running work, and can exit early if the +// result is not needed. +// +// void deepThoughtAsync(std::function Callback) { +// runAsync([Callback]{ +// int A = ponder(6); +// if (isCancelled()) +// return; +// int B = ponder(9); +// if (isCancelled()) +// return; +// Callback(A * B); +// }); +// } +// +// (A real example may invoke the callback with an error on cancellation, +// the CancelledError is provided for this purpose). +// +// Cancellation has some caveats: +// - the work will only stop when/if the library code next checks for it. +// Code outside clangd such as Sema will not do this. +// - it's inherently racy: client code must be prepared to accept results +// even after requesting cancellation. +// - it's Context-based, so async work must be dispatched to threads in +// ways that preserve the context. (Like runAsync() or TUScheduler). +// +// FIXME: We could add timestamps to isCancelled() and CancelledError. +// Measuring the start -> cancel -> acknowledge -> finish timeline would +// help find where libraries' cancellation should be improved. #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CANCELLATION_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CANCELLATION_H #include "Context.h" #include "llvm/Support/Error.h" -#include -#include +#include #include namespace clang { namespace clangd { -/// Enables signalling a cancellation on an async task or checking for -/// cancellation. It is thread-safe to trigger cancellation from multiple -/// threads or check for cancellation. Task object for the currently running -/// task can be obtained via clangd::getCurrentTask(). -class Task { -public: - void cancel() { CT = true; } - /// If cancellation checks are rare, one could use the isCancelled() helper in - /// the namespace to simplify the code. However, if cancellation checks are - /// frequent, the guideline is first obtain the Task object for the currently - /// running task with getCurrentTask() and do cancel checks using it to avoid - /// extra lookups in the Context. - bool isCancelled() const { return CT; } - - /// Creates a task handle that can be used by an async task to check for - /// information that can change during it's runtime, like Cancellation. - static std::shared_ptr createHandle() { - return std::shared_ptr(new Task()); - } - - Task(const Task &) = delete; - Task &operator=(const Task &) = delete; - Task(Task &&) = delete; - Task &operator=(Task &&) = delete; - -private: - Task() : CT(false) {} - std::atomic CT; -}; -using ConstTaskHandle = std::shared_ptr; -using TaskHandle = std::shared_ptr; - -/// Fetches current task information from Context. TaskHandle must have been -/// stashed into context beforehand. -const Task &getCurrentTask(); - -/// Stashes current task information within the context. -LLVM_NODISCARD Context setCurrentTask(ConstTaskHandle TH); - -/// Checks whether the current task has been cancelled or not. -/// Consider storing the task handler returned by getCurrentTask and then -/// calling isCancelled through it. getCurrentTask is expensive since it does a -/// lookup in the context. -inline bool isCancelled() { return getCurrentTask().isCancelled(); } +/// A canceller requests cancellation of a task, when called. +/// Calling it again has no effect. +using Canceler = std::function; + +/// Defines a new task whose cancellation may be requested. +/// The returned Context defines the scope of the task. +/// When the context is active, isCancelled() is false until the Canceler is +/// invoked, and true afterwards. +std::pair cancelableTask(); + +/// True if the current context is within a cancelable task which was cancelled. +/// Always false if there is no active cancelable task. +/// This isn't free (context lookup) - don't call it in a tight loop. +bool isCancelled(); +/// Conventional error when no result is returned due to cancellation. class CancelledError : public llvm::ErrorInfo { public: static char ID; Index: clangd/Cancellation.cpp =================================================================== --- clangd/Cancellation.cpp +++ clangd/Cancellation.cpp @@ -13,21 +13,21 @@ namespace clang { namespace clangd { -namespace { -static Key TaskKey; -} // namespace - char CancelledError::ID = 0; +static Key>> FlagKey; -const Task &getCurrentTask() { - const auto TH = Context::current().getExisting(TaskKey); - assert(TH && "Fetched a nullptr for TaskHandle from context."); - return *TH; +std::pair cancelableTask() { + auto Flag = std::make_shared>(); + return { + Context::current().derive(FlagKey, Flag), + [Flag] { *Flag = true; }, + }; } -Context setCurrentTask(ConstTaskHandle TH) { - assert(TH && "Trying to stash a nullptr as TaskHandle into context."); - return Context::current().derive(TaskKey, std::move(TH)); +bool isCancelled() { + if (auto *Flag = Context::current().get(FlagKey)) + return **Flag; + return false; // Not in scope of a task. } } // namespace clangd Index: clangd/ClangdLSPServer.h =================================================================== --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -170,9 +170,9 @@ // destructed instance of ClangdLSPServer. ClangdServer Server; - // Holds task handles for running requets. Key of the map is a serialized - // request id. - llvm::StringMap TaskHandles; + // Holds cancelers for running requets. Key of the map is a serialized + // request id. FIXME: handle cancellation generically in JSONRPCDispatcher. + llvm::StringMap TaskHandles; std::mutex TaskHandlesMutex; // Following three functions are for managing TaskHandles map. They store or @@ -183,7 +183,7 @@ // request. void CleanupTaskHandle(); void CreateSpaceForTaskHandle(); - void StoreTaskHandle(TaskHandle TH); + void StoreTaskHandle(Canceler TH); }; } // namespace clangd } // namespace clang Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -348,7 +348,7 @@ void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) { CreateSpaceForTaskHandle(); - TaskHandle TH = Server.codeComplete( + Canceler Cancel = Server.codeComplete( Params.textDocument.uri.file(), Params.position, CCOpts, [this](llvm::Expected List) { auto _ = llvm::make_scope_exit([this]() { CleanupTaskHandle(); }); @@ -361,7 +361,7 @@ LSPList.items.push_back(R.render(CCOpts)); return reply(std::move(LSPList)); }); - StoreTaskHandle(std::move(TH)); + StoreTaskHandle(std::move(Cancel)); } void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) { @@ -635,8 +635,7 @@ const auto &It = TaskHandles.find(Params.ID); if (It == TaskHandles.end()) return; - if (It->second) - It->second->cancel(); + It->second(); TaskHandles.erase(It); } @@ -659,7 +658,7 @@ elog("Creation of space for task handle: {0} failed.", NormalizedID); } -void ClangdLSPServer::StoreTaskHandle(TaskHandle TH) { +void ClangdLSPServer::StoreTaskHandle(Canceler TH) { const json::Value *ID = getRequestId(); if (!ID) return; Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -125,9 +125,9 @@ /// while returned future is not yet ready. /// A version of `codeComplete` that runs \p Callback on the processing thread /// when codeComplete results become available. - TaskHandle codeComplete(PathRef File, Position Pos, - const clangd::CodeCompleteOptions &Opts, - Callback CB); + Canceler codeComplete(PathRef File, Position Pos, + const clangd::CodeCompleteOptions &Opts, + Callback CB); /// Provide signature help for \p File at \p Pos. This method should only be /// called for tracked files. Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -201,16 +201,16 @@ WorkScheduler.remove(File); } -TaskHandle ClangdServer::codeComplete(PathRef File, Position Pos, - const clangd::CodeCompleteOptions &Opts, - Callback CB) { +Canceler ClangdServer::codeComplete(PathRef File, Position Pos, + const clangd::CodeCompleteOptions &Opts, + Callback CB) { // Copy completion options for passing them to async task handler. auto CodeCompleteOpts = Opts; if (!CodeCompleteOpts.Index) // Respect overridden index. CodeCompleteOpts.Index = Index; - TaskHandle TH = Task::createHandle(); - WithContext ContextWithCancellation(setCurrentTask(TH)); + auto Cancelable = cancelableTask(); + WithContext ContextWithCancellation(std::move(Cancelable.first)); // Copy PCHs to avoid accessing this->PCHs concurrently std::shared_ptr PCHs = this->PCHs; auto FS = FSProvider.getFileSystem(); @@ -259,7 +259,7 @@ // We use a potentially-stale preamble because latency is critical here. WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale, Bind(Task, File.str(), std::move(CB))); - return TH; + return std::move(Cancelable.second); } void ClangdServer::signatureHelp(PathRef File, Position Pos, Index: unittests/clangd/CancellationTests.cpp =================================================================== --- unittests/clangd/CancellationTests.cpp +++ unittests/clangd/CancellationTests.cpp @@ -13,57 +13,48 @@ namespace { TEST(CancellationTest, CancellationTest) { - TaskHandle TH = Task::createHandle(); - WithContext ContextWithCancellation(setCurrentTask(TH)); + auto Task = cancelableTask(); + WithContext ContextWithCancellation(std::move(Task.first)); EXPECT_FALSE(isCancelled()); - TH->cancel(); + Task.second(); EXPECT_TRUE(isCancelled()); } -TEST(CancellationTest, TaskTestHandleDiesContextLives) { +TEST(CancellationTest, CancelerDiesContextLives) { llvm::Optional ContextWithCancellation; { - TaskHandle TH = Task::createHandle(); - ContextWithCancellation.emplace(setCurrentTask(TH)); + auto Task = cancelableTask(); + ContextWithCancellation.emplace(std::move(Task.first)); EXPECT_FALSE(isCancelled()); - TH->cancel(); + Task.second(); EXPECT_TRUE(isCancelled()); } EXPECT_TRUE(isCancelled()); } TEST(CancellationTest, TaskContextDiesHandleLives) { - TaskHandle TH = Task::createHandle(); + auto Task = cancelableTask(); { - WithContext ContextWithCancellation(setCurrentTask(TH)); + WithContext ContextWithCancellation(std::move(Task.first)); EXPECT_FALSE(isCancelled()); - TH->cancel(); + Task.second(); EXPECT_TRUE(isCancelled()); } // Still should be able to cancel without any problems. - TH->cancel(); -} - -TEST(CancellationTest, CancellationToken) { - TaskHandle TH = Task::createHandle(); - WithContext ContextWithCancellation(setCurrentTask(TH)); - const auto &CT = getCurrentTask(); - EXPECT_FALSE(CT.isCancelled()); - TH->cancel(); - EXPECT_TRUE(CT.isCancelled()); + Task.second(); } TEST(CancellationTest, AsynCancellationTest) { std::atomic HasCancelled(false); Notification Cancelled; - auto TaskToBeCancelled = [&](ConstTaskHandle CT) { - WithContext ContextGuard(setCurrentTask(std::move(CT))); + auto TaskToBeCancelled = [&](Context Ctx) { + WithContext ContextGuard(std::move(Ctx)); Cancelled.wait(); HasCancelled = isCancelled(); }; - TaskHandle TH = Task::createHandle(); - std::thread AsyncTask(TaskToBeCancelled, TH); - TH->cancel(); + auto Task = cancelableTask(); + std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first)); + Task.second(); Cancelled.notify(); AsyncTask.join();