From 10f85fadfea0fade0642861116f7df79ee281e9a Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Tue, 12 Jun 2012 22:37:36 +0200 Subject: [PATCH] Fix child_process.kill oddities * When the process is already dead, but the `exit` signal wasn't raised yet, the ESRCH error should be ignored. * When an invalid signal is specified, kill() should throw. * Like process.kill(), child_process.kill() now preserves a `0` signal which can be used to check the liveliness of the child process. * process.kill() and child_process.kill() will now return true if the signal was actually delivered, and false otherwise. * When an `exec`-ed process is automatically killed because a time or buffer limit is exceeded, and the kill() fails, this error should be reported through the `exec` callback. Fixes: #3409 --- lib/child_process.js | 51 +++++++++++++++++++++++++++++++++++--------- src/node.js | 2 ++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 23465a706d1..48fc5de81cb 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -536,12 +536,24 @@ exports.execFile = function(file /* args, options, callback */) { } } + function errorhandler(e) { + err = e; + child.stdout.destroy(); + child.stderr.destroy(); + exithandler(); + } + function kill() { + child.stdout.destroy(); + child.stderr.destroy(); + killed = true; - child.kill(options.killSignal); - process.nextTick(function() { - exithandler(null, options.killSignal); - }); + try { + child.kill(options.killSignal); + } catch (e) { + err = e; + exithandler(); + } } if (options.timeout > 0) { @@ -571,6 +583,7 @@ exports.execFile = function(file /* args, options, callback */) { }); child.addListener('close', exithandler); + child.addListener('error', errorhandler); return child; }; @@ -822,25 +835,43 @@ function errnoException(errorno, syscall, errmsg) { ChildProcess.prototype.kill = function(sig) { + var signal; + if (!constants) { constants = process.binding('constants'); } - sig = sig || 'SIGTERM'; - var signal = constants[sig]; + if (sig === 0) { + signal = 0; + } else if (!sig) { + signal = constants['SIGTERM']; + } else { + signal = constants[sig]; + } - if (!signal) { + if (signal === undefined) { throw new Error('Unknown signal: ' + sig); } if (this._handle) { - this.killed = true; var r = this._handle.kill(signal); - if (r === -1) { + if (r == 0) { + /* Success. */ + this.killed = true; + return true; + } else if (errno == 'ESRCH') { + /* Already dead. */ + } else if (errno == 'EINVAL' || errno == 'ENOSYS') { + /* The underlying platform doesn't support this signal. */ + throw errnoException(errno, 'kill'); + } else { + /* Other error, almost certainly EPERM. */ this.emit('error', errnoException(errno, 'kill')); - return; } } + + /* Kill didn't succeed. */ + return false; }; diff --git a/src/node.js b/src/node.js index ba0786d6ea2..4571c7757db 100644 --- a/src/node.js +++ b/src/node.js @@ -430,6 +430,8 @@ if (r) { throw errnoException(errno, 'kill'); } + + return true; }; };