[Lldb-commits] [lldb] [lldb] Handle SIGINT via the MainLoop signal thread (on POSIX) (PR #195959)

[Jonas Devlieghere via lldb-commits]

JDevlieghere wrote:


> That's kind of true, but the current MainLoop implementation already does not guarantee you'll get a separate notification for each signal. Neither does the kernel (linux kernel, at least), for that matter. So like, if the user presses ^C twice really quickly (before we manage to run the signal callback), the second signal will not have any effect. That's why (pthread_)sigqueue exists, which we could theoretically use, but that would be pretty messy, so I wouldn't recommend it.
> 
> I guess my question is, why do you think the race is a problem. If the second ^C arrives while we're inside `DispatchInputInterrupt`, the process will get killed -- same as with this implementation. And if it arrives later (after we've restored the handler), then it will just cause us to run `DispatchInputInterrupt`, which shouldn't do anything (right?).

Yeah, fair enough. I think I'm convinced by your proposed SIGINT solution.

> Oh I see. The signal arrives between fork and execve. That makes sense, although I'm surprised that you can reproduce this reliably, as I would have expected this window to be very small. But then again, libSystem also appears do be doing a lot more post-fork work than I would have expected. This patch fixes that by making the signal handler innocuous in that context.

I may have overstated how reliably this reproduces. What helps is that we can spawn a lot of these in parallel, which helps increase the success rate. 

> BTW, does this mean that `DebugSymbols` is not using posix_spawn to launch the new process?

Unfortunately yes. Until recently, that's something my team owned, but I just handed it off :-) I filed a radar.

https://github.com/llvm/llvm-project/pull/195959