Fix signal handler#2152
Conversation
|
@mholt - any chance you could try this against your original issue (before you fixed it)? |
|
Good question, I've moved the |
stffabi
left a comment
There was a problem hiding this comment.
LGTM 🚀 , just added a minor comment.
| if (sigaction(signum, NULL, &st) < 0) { | ||
| goto fix_signal_error; | ||
| } | ||
| st.sa_flags |= SA_ONSTACK; |
There was a problem hiding this comment.
Should we skip fixup the signal handler if the SA_ONSTACK flag is already set?
There was a problem hiding this comment.
No harm in installing this one over it i guess? We know that either gtk or webkit do install one without SA_ONSTACK so we should be good regardless.
There was a problem hiding this comment.
Yeah absolutely, would maybe just save some cycles for internal setting up the handler in the kernel when it's already fine.But that can be neglected 😄
|
@leaanthony I will try this out ASAP! Is it OK if I run with |
|
Thanks for this fix @leaanthony - This was helpful for me in solving a pesky bug in pact-go where we consume a rust shared library, in various languages including golang via cgo. The issue only crops up where the rust shared library calls out to external plugins and manages their process lifecycle. |
|
I'm glad it was useful 🙏 |
This PR sets up a new signal handler in C that overrides the current one (in C) so that
SA_ONSTACKis used. When used against the nil dereference example @mholt gave, it appears to work as expected.Closes #2134