[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D52004#1232512, @kadircet wrote:

> Wonder if we can still keep the onCancelRequest registry within ProtocolHandler's scope, so that it is clear that we implement it. Other than that seems fascinating, thanks!


Hmm, I'm not sure how to do that while keeping things simple. I've updated the documentation on ClangdLSPServer and JSONRPCDispatcher to try to clarify instead. PTAL



================
Comment at: clangd/JSONRPCDispatcher.cpp:246
+  auto StrID = llvm::to_string(ID); // JSON-serialize ID for map key.
+  auto Cookie = RequestCookie++;
+  std::lock_guard<std::mutex> Lock(CancelMutex);
----------------
kadircet wrote:
> Maybe we can say something about this method is actually being invoked in a sync manner and the reason why we have mutex below is due to context destruction of the thread we might spawn later on. Because it bugged me at first look not having this line under mutex as well, then I noticed actually this was still a sync function.
Ah. Renamed this variable, commented the access, renamed the mutex to make it clear it's only for the map.
(The mutex itself has a comment about the fact that the *cleanups* are what happen off-thread)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52004