You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Shouldn't the default SIGINT handler trigger process.on('exit')? #2853
Maybe it's a bug, maybe it's by design. Documentation doesn't mention anything about whether or not the "exit" event should fire, but it's a bit silly I have to write:
process.on('SIGINT',function(){process.exit(somecode);// now the "exit" event will fire});
Just because the built-in handler won't do it for me. It also means I can no longer depend on the default exit code that Node associates with SIGINT, which apparently is 128 + signal number (according to the docs).
SIGINT from the terminal is supported on all platforms, and can usually be generated with CTRL+C
the docs describe a few other signals and continue:
Note that Windows does not support sending Signals
however, firing exit events on SIGINT should be an exception.
regardless of exit or uncaughtException listeners set on node processes, ctrl+c on terminal, should result in nothing else but immediate control returned to the terminal.
When node doesn't immediately exit.. I start to think there's either something wrong with my computer, something wrong with node.js being slow, my ssh ClientAliveInterval never got set in ~/.ssh/config so the terminal tab went all zombie and in like 7 or 8 minutes it might say broken pipe .
Reacted by Josh Junon, Alexey Timanovsky, Kirill Elagin, 浦东小林丹, Paul Erickson, Alex, -, Felix Divo and Leo
It seems to me that the fact that the exit event is not guaranteed to actually fire in situations like this should be mentioned in the docs. (Is it there and I'm just overlooking it? If not, I'm happy to put together a minimal PR for it. Probably only needs to be a sentence or two.) It would be great to be able to point to something official like that when people have problems like this.
@Trott nice PR, I just left you a comment on the diff. If you take a look at the section on Signal Events (which ought to be renamed to "Signal Interrupts" to help delineate the meaning of process events versus kernel/system signals), there's one ambiguity there I'd like to see clarified:
SIGINT from the terminal is supported on all platforms, and can usually be generated with CTRL+C
...
Note that Windows does not support sending Signals
The language here feels contradictory to me and it would be good to clarify that description as well.
well maybe that was written before windows 10 or whichever versions began to support such POSIX
btw, after pointing out how SIGHUP does the same, meaning there's more than three signals bypassing the exit event, i think we should be clear about that and include the rest of the list:
The issue he stated here calls for more fix than just the docs. The process module's exit event is borked until it only allows signal bypass from those we know to be intentional user input termination (ctrl+c).
SIGINT, SIGTERM, and SIGKILL cause the unconditional exit
if the exit event informed which signal is terming the process people wouldn't have to write:
process.on('SIGINT',function(){process.exit(somecode);// now the "exit" event will fire});
One exit handler would be more consistent with the idea of an exit event handler.
the only signal that an exit event should ignore is probably SIGINT or its equivalent on windows. I dont think we should allow the whole other assortment of termination signals, like SIGHUP for instance to bypass the exit event
SIGINT, SIGTERM, and SIGKILL cause the unconditional exit, those docs are wrong, they should say unconditional termination.
Processes terminate either normally, by exit, or by signal (on unix). The two forms of termination are different, and signal death isn't exit, and doesn't cause exit handlers to trigger. See http://linux.die.net/man/2/waitpid.
This code is correct, other than that it will cause the status to be exited instead of signalled:
process.on('SIGINT', function () {
process.exit(somecode); // now the "exit" event will fire
});
In particular, note that whether you want to clean up as if its a "normal" exit is a question specific to the application. You should decide what signals you want to trigger your cleanup handlers, and which signals you want to immediately kill the process. When you decide, listen to the ones you want to trap, as you do above.
On Windows, all this is emulated as best it can be, I tried to document the caveats, and what libuv does to try to emulate unix signals for some useful subset of behaviour, but the docs are never perfect.
>128 Signal Exits - If Node.js receives a fatal signal such as SIGKILL or SIGHUP,
then its exit code will be 128 plus the value of the signal code.
Those docs are wrong, too, for SIGKILL. SIGKILL is not trappable, and termination will be by signal.
That node traps some signals, cleans up the tty state, and then exits rather than re-terminating itself by a signal is a borderline bug that got fixed in 0.12, I thought, but mabye it still exists.
I don't know if I'll have the time to rework those docs in the next days, so if anyone wants to PR some improvements, please do!
I’m not particularly sure about the expected behaviour here, but fwiw this kind of question has come up a couple of times on the issue tracker here and people have been quite happy with accepting https://www.npmjs.com/package/signal-exit as a proper solution.
@Fishrock123 you re-opened, but its not clear from the conversation why, can it be closed? I think your dprof code link points to the wrong line now, can you update, and press y to convert the link to an absolute one?
This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.
regardless of exit or uncaughtException listeners set on node processes, ctrl+c on terminal, should result in nothing else but immediate control returned to the terminal.
Why? Which specification or best practice decrees this? Modern SIGINT is used to interrupt a process in order to gracefully exit it. Long-running processes are a very common use-case for Node.js and thus need a way to gracefully shutdown in a standard way (i.e. both manual and programmatic shutdowns). SIGINT is being used for that pretty much across the board.
14 million downloads of signal-exit seems like an indicator that this confusion isn't localized. Other languages have similar facilities (e.g. Python's atexit) and most all of them respond to SIGINT as a form of graceful exit signal.
I'm actually really surprised this isn't the case for Node.js.
Reacted by Alexey Timanovsky, Nathan Rajlich, Alex and Leo
importatexit@atexit.registerdefgoodbye():
print"You are now leaving the Python sector."importtimetime.sleep(100)
C/C++ don't have 'on process exit' handlers. They have signal handlers, which only allow you to catch specific signals. So those languages don't apply here.
Sorry, misread python docs. Interestingly, their atexit has the side effect of changing the return status, so you can't tell from the process status that it would have terminated by signal. Shrug.
I also think that's a poor metric for what is considered expected or correct.
I agree. It wasn't my argument, I was just contesting that node is unusual in following the lead of C's atexit(), or that if python does it, node should. People are free to use Node APIs to implement the behaviour they want, and are doing so successfully.
Activity
reqshark commentedon Sep 14, 2015
the docs describe a few other signals and continue:
however, firing exit events on
SIGINTshould be an exception.regardless of
exitoruncaughtExceptionlisteners set on node processes,ctrl+con terminal, should result in nothing else but immediate control returned to the terminal.When node doesn't immediately exit.. I start to think there's either something wrong with my computer, something wrong with node.js being slow, my ssh
ClientAliveIntervalnever got set in~/.ssh/configso the terminal tab went all zombie and in like 7 or 8 minutes it might say broken pipe .Trott commentedon Sep 14, 2015
It seems to me that the fact that the
exitevent is not guaranteed to actually fire in situations like this should be mentioned in the docs. (Is it there and I'm just overlooking it? If not, I'm happy to put together a minimal PR for it. Probably only needs to be a sentence or two.) It would be great to be able to point to something official like that when people have problems like this.reqshark commentedon Sep 14, 2015
@Trott nice PR, I just left you a comment on the diff. If you take a look at the section on Signal Events (which ought to be renamed to "Signal Interrupts" to help delineate the meaning of process events versus kernel/system signals), there's one ambiguity there I'd like to see clarified:
The language here feels contradictory to me and it would be good to clarify that description as well.
Trott commentedon Sep 14, 2015
Yeah, I'm not sure what that second line you quote is supposed to mean. It seems to be contradicted by https://msdn.microsoft.com/en-us/library/xdkz3x12.aspx.
reqshark commentedon Sep 14, 2015
well maybe that was written before windows 10 or whichever versions began to support such POSIX
btw, after pointing out how
SIGHUPdoes the same, meaning there's more than three signals bypassing the exit event, i think we should be clear about that and include the rest of the list:http://www.gnu.org/software/libc/manual/html_node/Termination-Signals.html
reqshark commentedon Sep 14, 2015
@ronkorving's right.
The issue he stated here calls for more fix than just the docs. The process module's exit event is borked until it only allows signal bypass from those we know to be intentional user input termination (ctrl+c).
IMO
reqshark commentedon Sep 15, 2015
the docs provide (and we accept) that:
if the exit event informed which signal is terming the process people wouldn't have to write:
One exit handler would be more consistent with the idea of an exit event handler.
the only signal that an exit event should ignore is probably
SIGINTor its equivalent on windows. I dont think we should allow the whole other assortment of termination signals, likeSIGHUPfor instance to bypass the exit eventsam-github commentedon Sep 16, 2015
SIGINT, SIGTERM, and SIGKILL cause the unconditional exit, those docs are wrong, they should say unconditional termination.Processes terminate either normally, by exit, or by signal (on unix). The two forms of termination are different, and signal death isn't exit, and doesn't cause exit handlers to trigger. See http://linux.die.net/man/2/waitpid.
This code is correct, other than that it will cause the status to be exited instead of signalled:
In particular, note that whether you want to clean up as if its a "normal" exit is a question specific to the application. You should decide what signals you want to trigger your cleanup handlers, and which signals you want to immediately kill the process. When you decide, listen to the ones you want to trap, as you do above.
On Windows, all this is emulated as best it can be, I tried to document the caveats, and what libuv does to try to emulate unix signals for some useful subset of behaviour, but the docs are never perfect.
sam-github commentedon Sep 16, 2015
Those docs are wrong, too, for SIGKILL. SIGKILL is not trappable, and termination will be by signal.
That node traps some signals, cleans up the tty state, and then exits rather than re-terminating itself by a signal is a borderline bug that got fixed in 0.12, I thought, but mabye it still exists.
I don't know if I'll have the time to rework those docs in the next days, so if anyone wants to PR some improvements, please do!
doc: process exit event is not guaranteed to fire
15 remaining items
Fishrock123 commentedon Sep 28, 2016
Ok unsure what I dug through and really can't be bothered to dig again, I'll just take your word on it.
¯_(ツ)_/¯
addaleax commentedon Sep 28, 2016
I’m not particularly sure about the expected behaviour here, but fwiw this kind of question has come up a couple of times on the issue tracker here and people have been quite happy with accepting https://www.npmjs.com/package/signal-exit as a proper solution.
Fishrock123 commentedon Sep 28, 2016
Yeah I had to work around this in dprof and was wondering why it didn't function that way...
addaleax commentedon Sep 28, 2016
Yeah, I can see why this is a tricky thing and why people would want to have the standard exit handler called…
bnoordhuis commentedon Feb 7, 2017
What is the consensus here? I see some changes have been made to the documentation. Can this issue be closed?
sam-github commentedon Feb 10, 2017
@Fishrock123 you re-opened, but its not clear from the conversation why, can it be closed? I think your dprof code link points to the wrong line now, can you update, and press
yto convert the link to an absolute one?Trott commentedon Jul 26, 2017
This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.
Qix- commentedon Nov 15, 2017
@reqshark
Why? Which specification or best practice decrees this? Modern SIGINT is used to interrupt a process in order to gracefully exit it. Long-running processes are a very common use-case for Node.js and thus need a way to gracefully shutdown in a standard way (i.e. both manual and programmatic shutdowns). SIGINT is being used for that pretty much across the board.
14 million downloads of
signal-exitseems like an indicator that this confusion isn't localized. Other languages have similar facilities (e.g. Python'satexit) and most all of them respond to SIGINT as a form of graceful exit signal.I'm actually really surprised this isn't the case for Node.js.
sam-github commentedon Nov 15, 2017
@qix python does not treat SIGINT as a normal exit:
https://docs.python.org/2/library/atexit.html
Nor does Ruby, or C/C++, or any other language I've ever worked with. Node.js is doing what everyone else does here.
Qix- commentedon Nov 16, 2017
@sam-github Python does indeed.
C/C++ don't have 'on process exit' handlers. They have signal handlers, which only allow you to catch specific signals. So those languages don't apply here.
Java also runs shutdown hooks upon Ctrl+C:
So no, Node.js is not doing "what everyone else does here". I also think that's a poor metric for what is considered expected or correct.
sam-github commentedon Nov 16, 2017
Sorry, misread python docs. Interestingly, their atexit has the side effect of changing the return status, so you can't tell from the process status that it would have terminated by signal. Shrug.
http://man7.org/linux/man-pages/man3/atexit.3.html
I agree. It wasn't my argument, I was just contesting that node is unusual in following the lead of C's
atexit(), or that if python does it, node should. People are free to use Node APIs to implement the behaviour they want, and are doing so successfully.Qix- commentedon Nov 16, 2017
TIL, thanks.
I think, at the very least, this should be documented with the
exitevent.sam-github commentedon Nov 16, 2017
The 'exit' event is emitted when the Node.js process is about to exit as a result of either:
The process.exit() method being called explicitly;
The Node.js event loop no longer having any additional work to perform.
^--- seems clear to me, but open an issue proposing the wording you think is missing, or even a PR if you have the inclination.