build: don't squash signal handlers with --shared - #10539
Conversation
Sorry, something went wrong.
|
Nit: it's->its and Currenty->Currently in the commit message |
Sorry, something went wrong.
|
Fixed (to current rather than currently) |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you provide a bit more detail on why its needed here in addition to the other location. The section that avoids restoring them because the parent may have changed them makes sense to me. But this part seems more related to the inspector and may still apply in the shared library case.
Sorry, something went wrong.
There was a problem hiding this comment.
I didn't think the inspector applied any additional signal handlers therefore it seems wrong to squash them all on the exit of the node runtime when loaded as a shared library (potentially someone could shutdown the node runtime within their own application and I wouldn't want this to squash any signal handlers in the caller).
I'm happy to be corrected though if there was a separate reason for this loop to be added in here. @eugeneo added this sequence in 6626919 so may have more info and be able to comment.
To be fair, as far as I can see this is only invoked when someone calls process.exit() from within javascript, in which case it'll currently be taking the wrapping process down anyway. on that basis I suspect it won't make a lot of practical difference either way, although I'd prefer to be sure that we really didn't want to squash the handlers in this "embedded" case before removing it.
Sorry, something went wrong.
|
ping @eugeneo regarding #10539 (comment) |
Sorry, something went wrong.
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM with a style nit.
Sorry, something went wrong.
There was a problem hiding this comment.
#endif // !NODE_SHARED_MODE (two spaces before the slashes)
Sorry, something went wrong.
There was a problem hiding this comment.
Done
Sorry, something went wrong.
Fixes: nodejs#10520 Ref: nodejs@dd47a8c 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.
|
CI: https://ci.nodejs.org/job/node-test-commit/7267/ (more of a sanity check really). Looks like Ben and @ofrobots reviewed 6626919, so unless @eugeneo or @ofrobots have any issue with #10539 (comment) EDIT: CI failed due to a full /tmp directory, rerunning: |
Sorry, something went wrong.
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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#10539 Fixes: nodejs#10520 Refs: nodejs#615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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: #10539 Fixes: #10520 Refs: #615 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Checklist
Affected core subsystem(s)
build
Fixes: #10520
Ref: dd47a8c
An application using node built as a shared library may legitimately
implement it's own signal handling routines. Currenty behaviour is
to squash all signal handlers on node startup. This change will
stop that behaviour when node is built as a shared library.
(Haven't yet checked with the person that raised the problem with me as they're currently on vacation, but this appears to resolve the problem as I understood it from them!)