labath wrote: > > I'm wondering if we could avoid grabbing another signal for this (SIGURG > > could in theory be used by some other part of the application). I think the > > same effect could be achieved by sending another SIGINT to the main thread > > (obviously, after we've restored the handler) -- and then setting a flag to > > ignore the next callback invocation. WDYT? > > Wouldn't that be race-y with a real external SIGINT, which would look > identical to the self-sent one? I guess we could check `siginfo_t.si_pid` in > the MainLoop to ignore it if it's our own PID? Let me know if you prefer this > over the `SIGURG` approach.
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?). > My experience with a library installing signal handlers has been nothing but > disastrous, so I'd be okay with preserving the current behavior and > limitations to avoid that. SGTM > The crash I'm looking at is a bit more involved than what I described in the > PR description. I don't think what I'm seeing is the problem you're > describing. This is what the crashlog looks like: 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. BTW, does this mean that `DebugSymbols` is not using posix_spawn to launch the new process? https://github.com/llvm/llvm-project/pull/195959 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
