[PATCH] D109506: [RFC] Print current request context along with the stack trance in clangd

[Sam McCall via Phabricator via llvm-commits]

sammccall added a comment.

Oops, forgot one thing: you probably want to instrument the preamble-building in TUScheduler (or in Preamble.cpp?), the background indexing, and code completion. Those all run the clang parser and should be a rich source of clang bugs.

Testing idea: `yes '[' | head -n 5000 | clangd --input-style=delimited` crashes the main thread, because the JSON parser is recursive...



================
Comment at: clang-tools-extra/clangd/support/ThreadSignalHandler.cpp:15
+
+static llvm::sys::ThreadLocal<ThreadSignalHandlerCallback> CurrentCallback;
+
----------------
sammccall wrote:
> sammccall wrote:
> > we assume `thread_local` in clangd (see support/Context.ccp)
> > 
> > It's possible that llvm::sys::ThreadLocal happens to play nicer with signal handlers in practice (none of these are completely safe!), but unless we've seen some concrete evidence I'd rather just have `static thread_local ThreadSignalHandlerCallback*` here
> we can quite easily support a stack of handlers, such that we dump *all* living handlers on the crashing thread.
> 
> just give each ThreadSignalHandler a `ThreadSignalHandler* Next = nullptr` member, and both the constructor and destructor `swap(Next, CurrentCallback)`. Then CurrentCallback points to the head of a linked list of handlers.
> 
> Not sure if you want to have that in this patch, but it's pretty simple so I wouldn't object.
Sorry, just realized you already suggested the stacking in the patch description!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109506/new/

https://reviews.llvm.org/D109506