sammccall updated this revision to Diff 165253.
sammccall marked 2 inline comments as done.
sammccall added a comment.
rebase and address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52004
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -56,7 +56,6 @@
virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
virtual void onHover(TextDocumentPositionParams &Params) = 0;
virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
- virtual void onCancelRequest(CancelParams &Params) = 0;
};
void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -76,5 +76,4 @@
Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
- Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
}
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -886,20 +886,6 @@
};
bool fromJSON(const llvm::json::Value &, ReferenceParams &);
-struct CancelParams {
- /// The request id to cancel.
- /// This can be either a number or string, if it is a number simply print it
- /// out and always use a string.
- std::string ID;
-};
-llvm::json::Value toJSON(const CancelParams &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &);
-bool fromJSON(const llvm::json::Value &, CancelParams &);
-
-/// Param can be either of type string or number. Returns the result as a
-/// string.
-llvm::Optional<std::string> parseNumberOrString(const llvm::json::Value *Param);
-
} // namespace clangd
} // namespace clang
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -623,38 +623,5 @@
return fromJSON(Params, Base);
}
-json::Value toJSON(const CancelParams &CP) {
- return json::Object{{"id", CP.ID}};
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &O, const CancelParams &CP) {
- O << toJSON(CP);
- return O;
-}
-
-llvm::Optional<std::string> parseNumberOrString(const json::Value *Params) {
- if (!Params)
- return llvm::None;
- // ID is either a number or a string, check for both.
- if(const auto AsString = Params->getAsString())
- return AsString->str();
-
- if(const auto AsNumber = Params->getAsInteger())
- return itostr(AsNumber.getValue());
-
- return llvm::None;
-}
-
-bool fromJSON(const json::Value &Params, CancelParams &CP) {
- const auto ParamsAsObject = Params.getAsObject();
- if (!ParamsAsObject)
- return false;
- if (auto Parsed = parseNumberOrString(ParamsAsObject->get("id"))) {
- CP.ID = std::move(*Parsed);
- return true;
- }
- return false;
-}
-
} // namespace clangd
} // namespace clang
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -10,6 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
+#include "Cancellation.h"
#include "Logger.h"
#include "Protocol.h"
#include "Trace.h"
@@ -79,21 +80,32 @@
/// registered Handler for the method received.
class JSONRPCDispatcher {
public:
- // A handler responds to requests for a particular method name.
+ /// A handler responds to requests for a particular method name.
+ ///
+ /// JSONRPCDispatcher will mark the handler's context as cancelled if a
+ /// matching cancellation request is received. Handlers are encouraged to
+ /// check for cancellation and fail quickly in this case.
using Handler = std::function<void(const llvm::json::Value &)>;
/// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
/// method is received.
- JSONRPCDispatcher(Handler UnknownHandler)
- : UnknownHandler(std::move(UnknownHandler)) {}
+ JSONRPCDispatcher(Handler UnknownHandler);
/// Registers a Handler for the specified Method.
void registerHandler(StringRef Method, Handler H);
/// Parses a JSONRPC message and calls the Handler for it.
- bool call(const llvm::json::Value &Message, JSONOutput &Out) const;
+ bool call(const llvm::json::Value &Message, JSONOutput &Out);
private:
+ // Tracking cancellations needs a mutex: handlers may finish on a different
+ // thread, and that's when we clean up entries in the map.
+ mutable std::mutex RequestCancelersMutex;
+ llvm::StringMap<std::pair<Canceler, unsigned>> RequestCancelers;
+ unsigned NextRequestCookie = 0;
+ Context cancelableRequestContext(const llvm::json::Value &ID);
+ void cancelRequest(const llvm::json::Value &ID);
+
llvm::StringMap<Handler> Handlers;
Handler UnknownHandler;
};
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -11,12 +11,14 @@
#include "Cancellation.h"
#include "ProtocolHandlers.h"
#include "Trace.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Chrono.h"
#include "llvm/Support/Errno.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/JSON.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/SourceMgr.h"
#include <istream>
@@ -158,6 +160,16 @@
});
}
+JSONRPCDispatcher::JSONRPCDispatcher(Handler UnknownHandler)
+ : UnknownHandler(std::move(UnknownHandler)) {
+ registerHandler("$/cancelRequest", [this](const json::Value &Params) {
+ if (auto *O = Params.getAsObject())
+ if (auto *ID = O->get("id"))
+ return cancelRequest(*ID);
+ log("Bad cancellation request: {0}", Params);
+ });
+}
+
void JSONRPCDispatcher::registerHandler(StringRef Method, Handler H) {
assert(!Handlers.count(Method) && "Handler already registered!");
Handlers[Method] = std::move(H);
@@ -180,8 +192,7 @@
}
}
-bool JSONRPCDispatcher::call(const json::Value &Message,
- JSONOutput &Out) const {
+bool JSONRPCDispatcher::call(const json::Value &Message, JSONOutput &Out) {
// Message must be an object with "jsonrpc":"2.0".
auto *Object = Message.getAsObject();
if (!Object || Object->getString("jsonrpc") != Optional<StringRef>("2.0"))
@@ -214,12 +225,47 @@
SPAN_ATTACH(Tracer, "ID", *ID);
SPAN_ATTACH(Tracer, "Params", Params);
+ // Requests with IDs can be canceled by the client. Add cancellation context.
+ llvm::Optional<WithContext> WithCancel;
+ if (ID)
+ WithCancel.emplace(cancelableRequestContext(*ID));
+
// Stash a reference to the span args, so later calls can add metadata.
WithContext WithRequestSpan(RequestSpan::stash(Tracer));
Handler(std::move(Params));
return true;
}
+// We run cancelable requests in a context that does two things:
+// - allows cancellation using RequestCancelers[ID]
+// - cleans up the entry in RequestCancelers when it's no longer needed
+// If a client reuses an ID, the last one wins and the first cannot be canceled.
+Context JSONRPCDispatcher::cancelableRequestContext(const json::Value &ID) {
+ auto Task = cancelableTask();
+ auto StrID = llvm::to_string(ID); // JSON-serialize ID for map key.
+ auto Cookie = NextRequestCookie++; // No lock, only called on main thread.
+ {
+ std::lock_guard<std::mutex> Lock(RequestCancelersMutex);
+ RequestCancelers[StrID] = {std::move(Task.second), Cookie};
+ }
+ // When the request ends, we can clean up the entry we just added.
+ // The cookie lets us check that it hasn't been overwritten due to ID reuse.
+ return Task.first.derive(make_scope_exit([this, StrID, Cookie] {
+ std::lock_guard<std::mutex> Lock(RequestCancelersMutex);
+ auto It = RequestCancelers.find(StrID);
+ if (It != RequestCancelers.end() && It->second.second == Cookie)
+ RequestCancelers.erase(It);
+ }));
+}
+
+void JSONRPCDispatcher::cancelRequest(const json::Value &ID) {
+ auto StrID = llvm::to_string(ID);
+ std::lock_guard<std::mutex> Lock(RequestCancelersMutex);
+ auto It = RequestCancelers.find(StrID);
+ if (It != RequestCancelers.end())
+ It->second.first(); // Invoke the canceler.
+}
+
// Tries to read a line up to and including \n.
// If failing, feof() or ferror() will be set.
static bool readLine(std::FILE *In, std::string &Out) {
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -45,15 +45,25 @@
std::vector<Diag> Diagnostics) = 0;
};
-/// Provides API to manage ASTs for a collection of C++ files and request
-/// various language features.
-/// Currently supports async diagnostics, code completion, formatting and goto
-/// definition.
+/// Manages a collection of source files and derived data (ASTs, indexes),
+/// and provides language-aware features such as code completion.
+///
+/// The primary client is ClangdLSPServer which exposes these features via
+/// the Language Server protocol. ClangdServer may also be embedded directly,
+/// though its API is not stable over time.
+///
+/// ClangdServer should be used from a single thread. Many potentially-slow
+/// operations have asynchronous APIs and deliver their results on another
+/// thread.
+/// Such operations support cancellation: if the caller sets up a cancelable
+/// context, many operations will notice cancellation and fail early.
+/// (ClangdLSPServer uses this to implement $/cancelRequest).
class ClangdServer {
public:
struct Options {
/// To process requests asynchronously, ClangdServer spawns worker threads.
- /// If 0, all requests are processed on the calling thread.
+ /// If this is zero, no threads are spawned. All work is done on the calling
+ /// thread, and callbacks are invoked before "async" functions return.
unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
/// AST caching policy. The default is to keep up to 3 ASTs in memory.
@@ -125,9 +135,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.
- Canceler codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions &Opts,
- Callback<CodeCompleteResult> CB);
+ void codeComplete(PathRef File, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
+ Callback<CodeCompleteResult> 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
@@ -8,7 +8,6 @@
//===-------------------------------------------------------------------===//
#include "ClangdServer.h"
-#include "Cancellation.h"
#include "CodeComplete.h"
#include "FindSymbols.h"
#include "Headers.h"
@@ -201,16 +200,14 @@
WorkScheduler.remove(File);
}
-Canceler ClangdServer::codeComplete(PathRef File, Position Pos,
- const clangd::CodeCompleteOptions &Opts,
- Callback<CodeCompleteResult> CB) {
+void ClangdServer::codeComplete(PathRef File, Position Pos,
+ const clangd::CodeCompleteOptions &Opts,
+ Callback<CodeCompleteResult> CB) {
// Copy completion options for passing them to async task handler.
auto CodeCompleteOpts = Opts;
if (!CodeCompleteOpts.Index) // Respect overridden index.
CodeCompleteOpts.Index = Index;
- auto Cancelable = cancelableTask();
- WithContext ContextWithCancellation(std::move(Cancelable.first));
// Copy PCHs to avoid accessing this->PCHs concurrently
std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
auto FS = FSProvider.getFileSystem();
@@ -259,7 +256,6 @@
// 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 std::move(Cancelable.second);
}
void ClangdServer::signatureHelp(PathRef File, Position Pos,
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -76,7 +76,6 @@
void onRename(RenameParams &Parames) override;
void onHover(TextDocumentPositionParams &Params) override;
void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
- void onCancelRequest(CancelParams &Params) override;
std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
@@ -169,21 +168,6 @@
// the worker thread that may otherwise run an async callback on partially
// destructed instance of ClangdLSPServer.
ClangdServer Server;
-
- // Holds cancelers for running requets. Key of the map is a serialized
- // request id. FIXME: handle cancellation generically in JSONRPCDispatcher.
- llvm::StringMap<Canceler> TaskHandles;
- std::mutex TaskHandlesMutex;
-
- // Following three functions are for managing TaskHandles map. They store or
- // remove a task handle for the request-id stored in current Context.
- // FIXME(kadircet): Wrap the following three functions in a RAII object to
- // make sure these do not get misused. The object might be stored in the
- // Context of the thread or moved around until a reply is generated for the
- // request.
- void CleanupTaskHandle();
- void CreateSpaceForTaskHandle();
- void StoreTaskHandle(Canceler TH);
};
} // namespace clangd
} // namespace clang
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -8,7 +8,6 @@
//===----------------------------------------------------------------------===//
#include "ClangdLSPServer.h"
-#include "Cancellation.h"
#include "Diagnostics.h"
#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
@@ -71,11 +70,6 @@
return Defaults;
}
-std::string NormalizeRequestID(const json::Value &ID) {
- auto NormalizedID = parseNumberOrString(&ID);
- assert(NormalizedID && "Was not able to parse request id.");
- return std::move(*NormalizedID);
-}
} // namespace
void ClangdLSPServer::onInitialize(InitializeParams &Params) {
@@ -347,21 +341,16 @@
}
void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {
- CreateSpaceForTaskHandle();
- Canceler Cancel = Server.codeComplete(
- Params.textDocument.uri.file(), Params.position, CCOpts,
- [this](llvm::Expected<CodeCompleteResult> List) {
- auto _ = llvm::make_scope_exit([this]() { CleanupTaskHandle(); });
-
- if (!List)
- return replyError(List.takeError());
- CompletionList LSPList;
- LSPList.isIncomplete = List->HasMore;
- for (const auto &R : List->Completions)
- LSPList.items.push_back(R.render(CCOpts));
- return reply(std::move(LSPList));
- });
- StoreTaskHandle(std::move(Cancel));
+ Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
+ [this](llvm::Expected<CodeCompleteResult> List) {
+ if (!List)
+ return replyError(List.takeError());
+ CompletionList LSPList;
+ LSPList.isIncomplete = List->HasMore;
+ for (const auto &R : List->Completions)
+ LSPList.items.push_back(R.render(CCOpts));
+ return reply(std::move(LSPList));
+ });
}
void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
@@ -629,48 +618,3 @@
return *CachingCDB;
return *CDB;
}
-
-void ClangdLSPServer::onCancelRequest(CancelParams &Params) {
- std::lock_guard<std::mutex> Lock(TaskHandlesMutex);
- const auto &It = TaskHandles.find(Params.ID);
- if (It == TaskHandles.end())
- return;
- It->second();
- TaskHandles.erase(It);
-}
-
-void ClangdLSPServer::CleanupTaskHandle() {
- const json::Value *ID = getRequestId();
- if (!ID)
- return;
- std::string NormalizedID = NormalizeRequestID(*ID);
- std::lock_guard<std::mutex> Lock(TaskHandlesMutex);
- TaskHandles.erase(NormalizedID);
-}
-
-void ClangdLSPServer::CreateSpaceForTaskHandle() {
- const json::Value *ID = getRequestId();
- if (!ID)
- return;
- std::string NormalizedID = NormalizeRequestID(*ID);
- std::lock_guard<std::mutex> Lock(TaskHandlesMutex);
- if (!TaskHandles.insert({NormalizedID, nullptr}).second)
- elog("Creation of space for task handle: {0} failed.", NormalizedID);
-}
-
-void ClangdLSPServer::StoreTaskHandle(Canceler TH) {
- const json::Value *ID = getRequestId();
- if (!ID)
- return;
- std::string NormalizedID = NormalizeRequestID(*ID);
- std::lock_guard<std::mutex> Lock(TaskHandlesMutex);
- auto It = TaskHandles.find(NormalizedID);
- if (It == TaskHandles.end()) {
- elog("CleanupTaskHandle called before store can happen for request:{0}.",
- NormalizedID);
- return;
- }
- if (It->second != nullptr)
- elog("TaskHandle didn't get cleared for: {0}.", NormalizedID);
- It->second = std::move(TH);
-}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
