From 06b6e75668ae9b4f57a61d5b48633ae7d3f8f434 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Mon, 29 Dec 2014 18:00:02 -0800 Subject: [PATCH] process: removeAllListeners failing for 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 56668f54d1. That causes 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') --- src/node.js | 2 + .../process-remove-all-signal-listeners.js | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 test/simple/process-remove-all-signal-listeners.js diff --git a/src/node.js b/src/node.js index e95a01fb4924..7dce99eb797d 100644 --- a/src/node.js +++ b/src/node.js @@ -767,6 +767,8 @@ var addListener = process.addListener; var removeListener = process.removeListener; + process.on('removeListener', function() {}); + function isSignal(event) { return event.slice(0, 3) === 'SIG' && startup.lazyConstants().hasOwnProperty(event); diff --git a/test/simple/process-remove-all-signal-listeners.js b/test/simple/process-remove-all-signal-listeners.js new file mode 100644 index 000000000000..23aa22decfc7 --- /dev/null +++ b/test/simple/process-remove-all-signal-listeners.js @@ -0,0 +1,41 @@ +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.env.DOTEST) { + // 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]); + + child.once('exit', function(code, signal) { + assert.equal(signal, 'SIGINT'); + ok = true; + }); + + process.on('exit', function(code) { + if (code === 0) + 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');