-
Notifications
You must be signed in to change notification settings - Fork 29.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
process: fix regression in unlistening signals
When the last signal listener is removed, the signal wrap should be closed, restoring the default signal handling behaviour. This is done in a (patched) process.removeListener(). However, events.removeAllListeners has an optimization to avoid calling removeListener() if there are no listeners for the 'removeListener' event, introduced in 56668f5. That caused the following code to fail to terminate: process.stdin.resume(); function listener() {}; process.on('SIGINT', listener); process.removeAllListeners('SIGINT'); process.kill(process.pid, 'SIGINT') while the following will terminate: process.stdin.resume(); function listener() {}; process.on('SIGINT', listener); process.removeListener('SIGINT', listener); process.kill(process.pid, 'SIGINT') Replace the method patching with use of the 'newListener' and 'removeListener' events, which will fire no matter which methods are used to add or remove listeners.
- Loading branch information
1 parent
e53bd67
commit 09653fb
Showing
2 changed files
with
49 additions
and
19 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
if (process.platform === 'win32') { | ||
// Win32 doesn't have signals, just a kindof emulation, insufficient | ||
// for this test to apply. | ||
return; | ||
} | ||
|
||
var assert = require('assert'); | ||
var spawn = require('child_process').spawn; | ||
var ok; | ||
|
||
if (process.argv[2] !== '--do-test') { | ||
// We are the master, fork a child so we can verify it exits with correct | ||
// status. | ||
process.env.DOTEST = 'y'; | ||
var child = spawn(process.execPath, [__filename, '--do-test']); | ||
|
||
child.once('exit', function(code, signal) { | ||
assert.equal(signal, 'SIGINT'); | ||
ok = true; | ||
}); | ||
|
||
process.on('exit', function() { | ||
assert(ok); | ||
}); | ||
|
||
return; | ||
} | ||
|
||
process.on('SIGINT', function() { | ||
// Remove all handlers and kill ourselves. We should terminate by SIGINT | ||
// now that we have no handlers. | ||
process.removeAllListeners('SIGINT'); | ||
process.kill(process.pid, 'SIGINT'); | ||
}); | ||
|
||
// Signal handlers aren't sufficient to keep node alive, so resume stdin | ||
process.stdin.resume(); | ||
|
||
// Demonstrate that signals are being handled | ||
process.kill(process.pid, 'SIGINT'); |