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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to