Skip to content

Add SA_RESETHAND to SIGSEGV handler to prevent infinite signal loop#487

Closed
yoochangyeon wants to merge 1 commit into
DataDog:mainfrom
yoochangyeon:fix/sigsegv-handler-safety
Closed

Add SA_RESETHAND to SIGSEGV handler to prevent infinite signal loop#487
yoochangyeon wants to merge 1 commit into
DataDog:mainfrom
yoochangyeon:fix/sigsegv-handler-safety

Conversation

@yoochangyeon

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds SA_RESETHAND flag to the SIGSEGV signal handler registration in src/ddprof.cc and test/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 will
terminate 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 strace output 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 (readwritert_sigreturn) did not match ddprof's sigsegv_handler (which calls write to stderr and _exit). This indicates that another signal handler (likely from the Node.js runtime) had overridden
ddprof'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_RESETHAND prevents 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

How to test the change?

  1. Existing tests: The change only adds a flag to sigaction() and does not alter handler logic, so existing tests should pass without modification.
  2. Manual verification: Attach ddprof to a process alongside another runtime (e.g., Node.js) that also installs a SIGSEGV handler. Trigger a SIGSEGV — the process should terminate instead of entering an infinite loop.
  3. Flag verification: Run cat /proc/<pid>/status | grep SigCgt to confirm SIGSEGV (signal 11) is still caught after the change.

@r1viollet

Copy link
Copy Markdown
Collaborator

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.

@r1viollet

r1viollet commented Feb 12, 2026

Copy link
Copy Markdown
Collaborator

Adding the changes to #488 (this allows CI to run fully).

@r1viollet r1viollet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@r1viollet

Copy link
Copy Markdown
Collaborator

Closing as this is merged in #488 , however I doubt that this caused the issue you were observing.

@r1viollet r1viollet closed this Feb 12, 2026
@yoochangyeon

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing and merging this into #488!

Your question was spot on:

"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."

You're right

I had a fundamental misunderstanding of the architecture.
I assumed ddprof's handler was operating within the Node.js process,
but it's a separate profiling process with its own independent signal handler.

The infinite SIGSEGV loop I observed was happening entirely within the
Node.js process itself - its own handler was catching SIGSEGV and returning
via rt_sigreturn, causing the faulting instruction to re-execute endlessly.

Your question was what helped me realize the flawed assumption.
Thanks for taking the time to not just review the code, but question
the underlying premise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants