[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 1 02:34:13 PDT 2019

sammccall added inline comments.

Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:266
         elog("No reply to message {0}({1})", Method, ID);
         assert(false && "must reply to all calls!");
         (*this)(llvm::make_error<LSPError>("server failed to reply",
hokein wrote:
> hmm, we will trigger this assertion when clangd exits but not all requests are replied by the client, options:
> 1) remove the assertion here, but the error message "server failed to reply" returned is misleading, in this case, this is a client failure, not server
> 2) make sure that all reply callbacks are called, e.g. run the pending reply callback in destructor (returning an LSP error to client)
The latter is pretty, but may be complicated (shutdown protocol is messy and involves receiving multiple messages, what happens if a reply arrives in between?)

I would suggest changing the condition here: if the server's destructor has started then don't attempt to reply. You can have an atomic<bool> in ClangdLSPServer that's set at the top of the destructor

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list