Add SA_RESETHAND to SIGSEGV handler to prevent infinite signal loop#487
Add SA_RESETHAND to SIGSEGV handler to prevent infinite signal loop#487yoochangyeon wants to merge 1 commit into
Conversation
|
Thanks for the contribution. I think that the change is fine. I would possibly have fixed the return on write failure (avoiding a return and consolidating to have a single exit path). On the description, something feel strange: the handler is only installed in ddprof which runs in a separate process, so I have trouble understanding how Node's handler could somehow interact with this handler. |
|
Adding the changes to #488 (this allows CI to run fully). |
|
Closing as this is merged in #488 , however I doubt that this caused the issue you were observing. |
|
Thanks for reviewing and merging this into #488! Your question was spot on:
You're right I had a fundamental misunderstanding of the architecture. The infinite SIGSEGV loop I observed was happening entirely within the Your question was what helped me realize the flawed assumption. |
What does this PR do?
Adds
SA_RESETHANDflag to the SIGSEGV signal handler registration insrc/ddprof.ccandtest/simple_malloc.cc.With this flag, the kernel automatically resets the signal disposition to
SIG_DFL(default behavior: process termination) before entering the handler. This ensures that if the handler fails to reach_exit(), the next SIGSEGV willterminate the process instead of re-entering the handler infinitely.
Motivation
We observed a production issue where a Node.js process profiled by ddprof entered an infinite SIGSEGV loop, causing 100% CPU usage that never recovered.
The
straceoutput showed this pattern repeating indefinitely:[pid 2881] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xe3a8} ---
[pid 2881] read(7, "", 1) = 1
[pid 2881] write(22, "...", 16) = -1 EAGAIN (Resource temporarily unavailable)
[pid 2881] write(8, "", 1) = 1
[pid 2881] rt_sigreturn({mask=[]}) = 0
The syscall pattern (
read→write→rt_sigreturn) did not match ddprof'ssigsegv_handler(which callswriteto stderr and_exit). This indicates that another signal handler (likely from the Node.js runtime) had overriddenddprof's handler via a later
sigaction()call.Because the overriding handler returns normally via
rt_sigreturn, the CPU re-executes the faulting instruction, which triggers SIGSEGV again — creating an infinite loop.SA_RESETHANDprevents this scenario: even if ddprof's handler is overridden, or if the handler itself fails to terminate the process, the second SIGSEGV will use the default disposition and terminate the process cleanly.Additional Notes
SA_RESETHANDis POSIX-standard and supported on all Linux versions._exit()). It only prevents the infinite loop edge case.test/simple_malloc.ccfor consistency.How to test the change?
sigaction()and does not alter handler logic, so existing tests should pass without modification.cat /proc/<pid>/status | grep SigCgtto confirm SIGSEGV (signal 11) is still caught after the change.