Fix child_process.kill oddities

* Failure should be reported by throwing, like process.kill(). * `exec`-ed processes should not blow up when, upon reaching time / buffer limit, an attempt is made to kill them, and this kill fails because the process already exited. * Document this behaviour. Fixes: #3409
nodejs · Jun 12, 2012 · 486a5b3 · isaacs · Jun 12, 2012 · 486a5b3
1 parent a55faea
commit 486a5b3
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/doc/api/child_process.markdown b/doc/api/child_process.markdown
@@ -119,8 +119,14 @@ be sent `'SIGTERM'`. See `signal(7)` for a list of available signals.
     // send SIGHUP to process
     grep.kill('SIGHUP');
 
-Note that while the function is called `kill`, the signal delivered to the child
-process may not actually kill it.  `kill` really just sends a signal to a process.
+Although the function is called `kill`, the signal delivered to the child
+process may not actually kill it.  `kill` really just sends a signal to a
+process.
+
+Also note that if the child process already exited when `kill` is called,
+an exception will be thrown. This can even happen when the `child_process`
+object did not emit `exit yet. Therefore it is wise to wrap the `kill` call in
+a try..catch block.
 
 See `kill(2)`
 

diff --git a/lib/child_process.js b/lib/child_process.js
@@ -538,10 +538,12 @@ exports.execFile = function(file /* args, options, callback */) {
 
   function kill() {
     killed = true;
-    child.kill(options.killSignal);
-    process.nextTick(function() {
-      exithandler(null, options.killSignal);
-    });
+    try {
+      child.kill(options.killSignal);
+    } catch (e) {
+    }
+    child.stdout.destroy();
+    child.stderr.destroy();
   }
 
   if (options.timeout > 0) {
@@ -837,8 +839,7 @@ ChildProcess.prototype.kill = function(sig) {
     this.killed = true;
     var r = this._handle.kill(signal);
     if (r === -1) {
-      this.emit('error', errnoException(errno, 'kill'));
-      return;
+      throw errnoException(errno, 'kill');
     }
   }
 };