src: rework early debug signal handling#615
Merged
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Contributor
sam-github commented Jan 26, 2015
LGTM, and thanks. I agree that setting all the sighandlers to default is a good idea, SIG_IGN is inheritable across exec. For the record, I noticed this problem with a bug in some of my code that was masking out SIGCHLD until ppoll(), but I forgot that after forking I'd have to clear the procmask before execing node as a child process. That caused uv to never received SIGCHLD, and so to never notice child process exit. |
Execute the per-platform initialization logic as early as possible, for two reasons: 1. It opens the way for an upcoming commit to simplify early SIGUSR1 handling. 2. It should make life easier for embedders because io.js no longer mucks around with the file descriptor limit or signal disposition of the process. PR-URL: nodejs#615 Reviewed-By: Sam Roberts <[email protected]>
Instead of installing an early debug signal handler, simply block the SIGUSR1 signal at start-up and unblock it when the debugger is ready. Both approaches are functionally equivalent but blocking the signal accomplishes it in fewer lines of code. PR-URL: nodejs#615 Reviewed-By: Sam Roberts <[email protected]>
Signal dispositions are inherited by child processes. Restore ours to sane defaults in case our parent process changed it, to prevent quirky behavior when the parent does something silly like ignoring SIGSEGV. PR-URL: nodejs#615 Reviewed-By: Sam Roberts <[email protected]>
bnoordhuisforce-pushed the rework-signal-handling branch from f64f97d to dd47a8cCompare
gibfahn pushed a commit that referenced this pull request Jan 17, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
sxa pushed a commit to sxa/node that referenced this pull request Jan 18, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 23, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 25, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
sxa pushed a commit to sxa/node that referenced this pull request Feb 1, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: nodejs#10539Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
An application using node built as a shared library may legitimately implement its own signal handling routines. Current behaviour is to squash all signal handlers on node startup. This change will stop that behaviour when node is built as a shared library. PR-URL: #10539Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of installing an early debug signal handler, simply block the
SIGUSR1 signal at start-up and unblock it when the debugger is ready.
Both approaches are functionally equivalent but blocking the signal
accomplishes it in fewer lines of code.
R=@sam-github? It's not necessary to SIG_DFL SIGCHLD because
libuv installs its own handler when it spawns a process, but it does
seem like a good idea to explicitly install default handlers.
https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/116/