Make sure Error object on exec() gets killed member

Also default to SIGTERM for destruction when exceeding timeout or buffer on
exec()

Back ported from v0.3; original commits:
bd8e4f656e0cdbcf98a0a2cfe0c7b41645b56810
5a98fa4809457f843aae5447ce82b660c8b595f3
6570cd99e51adb082d0e0076a1205cd867f4ce41
9bf2975f78ce097c82a720bdda82810c1188a8ef

3 files changed, 89 insertions, 31 deletions

diff --git a/doc/api.markdown b/doc/api.markdown
index 75f10bd2a..e48ef2073 100644
--- a/doc/api.markdown
+++ b/doc/api.markdown
@@ -1055,14 +1055,14 @@ There is a second optional argument to specify several options. The default opti
     { encoding: 'utf8'
     , timeout: 0
     , maxBuffer: 200*1024
-    , killSignal: 'SIGKILL'
+    , killSignal: 'SIGTERM'
     , cwd: null
     , env: null
     }
 
 If `timeout` is greater than 0, then it will kill the child process
 if it runs longer than `timeout` milliseconds. The child process is killed with
-`killSignal` (default: `'SIGKILL'`). `maxBuffer` specifies the largest
+`killSignal` (default: `'SIGTERM'`). `maxBuffer` specifies the largest
 amount of data allowed on stdout or stderr - if this value is exceeded then
 the child process is killed.
 
diff --git a/lib/child_process.js b/lib/child_process.js
index fb5c7fd8d..4e6547b42 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -27,7 +27,7 @@ exports.execFile = function (file /* args, options, callback */) {
   var options = { encoding: 'utf8'
                 , timeout: 0
                 , maxBuffer: 200*1024
-                , killSignal: 'SIGKILL'
+                , killSignal: 'SIGTERM'
                 , cwd: null
                 , env: null
                 };
@@ -60,15 +60,43 @@ exports.execFile = function (file /* args, options, callback */) {
   var stdout = "";
   var stderr = "";
   var killed = false;
-
+  var exited = false;
   var timeoutId;
+
+  function exithandler (code, signal) {
+    if (exited) return;
+    exited = true;
+
+    if (timeoutId) {
+      clearTimeout(timeoutId);
+      timeoutId = null;
+    }
+
+    if (!callback) return;
+
+    if (code === 0 && signal === null) {
+      callback(null, stdout, stderr);
+    } else {
+      var e = new Error("Command failed: " + stderr);
+      e.killed = child.killed || killed;
+      e.code = code;
+      e.signal = signal;
+      callback(e, stdout, stderr);
+    }
+  }
+
+  function kill () {
+    killed = true;
+    child.kill(options.killSignal);
+    process.nextTick(function () {
+      exithandler(null, options.killSignal)
+    });
+  }
+
   if (options.timeout > 0) {
     timeoutId = setTimeout(function () {
-      if (!killed) {
-        child.kill(options.killSignal);
-        killed = true;
-        timeoutId = null;
-      }
+      kill();
+      timeoutId = null;
     }, options.timeout);
   }
 
@@ -77,32 +105,19 @@ exports.execFile = function (file /* args, options, callback */) {
 
   child.stdout.addListener("data", function (chunk) {
     stdout += chunk;
-    if (!killed && stdout.length > options.maxBuffer) {
-      child.kill(options.killSignal);
-      killed = true;
+    if (stdout.length > options.maxBuffer) {
+      kill();
     }
   });
 
   child.stderr.addListener("data", function (chunk) {
     stderr += chunk;
-    if (!killed && stderr.length > options.maxBuffer) {
-      child.kill(options.killSignal);
-      killed = true
+    if (stderr.length > options.maxBuffer) {
+      kill();
     }
   });
 
-  child.addListener("exit", function (code, signal) {
-    if (timeoutId) clearTimeout(timeoutId);
-    if (code === 0 && signal === null) {
-      if (callback) callback(null, stdout, stderr);
-    } else {
-      var e = new Error("Command failed: " + stderr);
-      e.killed = killed;
-      e.code = code;
-      e.signal = signal;
-      if (callback) callback(e, stdout, stderr);
-    }
-  });
+  child.addListener("exit", exithandler);
 
   return child;
 };
@@ -113,6 +128,8 @@ function ChildProcess () {
 
   var self = this;
 
+  this.killed = false;
+
   var gotCHLD = false;
   var exitCode;
   var termSignal;
@@ -158,7 +175,11 @@ inherits(ChildProcess, EventEmitter);
 
 
 ChildProcess.prototype.kill = function (sig) {
-  return this._internal.kill(sig);
+  if (this._internal.pid) {
+    this.killed = true;
+    sig = sig || 'SIGTERM';
+    return this._internal.kill(sig);
+  }
 };
 
 
diff --git a/test/simple/test-exec.js b/test/simple/test-exec.js
index c37d0ddcd..012d595d6 100644
--- a/test/simple/test-exec.js
+++ b/test/simple/test-exec.js
@@ -35,16 +35,53 @@ exec("ls /DOES_NOT_EXIST", function (err, stdout, stderr) {
   }
 });
 
-exec("sleep 10", { timeout: 50 }, function (err, stdout, stderr) {
+
+
+var sleeperStart = new Date();
+exec("sleep 3", { timeout: 50 }, function (err, stdout, stderr) {
+  var diff = (new Date()) - sleeperStart;
+  console.log("sleep 3 with timeout 50 took %d ms", diff);
+  assert.ok(diff < 500);
   assert.ok(err);
   assert.ok(err.killed);
-  assert.equal(err.signal, 'SIGKILL');
+  assert.equal(err.signal, 'SIGTERM');
+});
+
+
+
+
+var startSleep3 = new Date();
+var killMeTwice = exec("sleep 3", { timeout: 1000 }, killMeTwiceCallback);
+
+process.nextTick(function(){
+  console.log("kill pid %d", killMeTwice.pid); 
+  // make sure there is no race condition in starting the process
+  // the PID SHOULD exist directly following the exec() call.
+  assert.equal('number', typeof killMeTwice._internal.pid);
+  // Kill the process 
+  killMeTwice.kill();
 });
 
+function killMeTwiceCallback(err, stdout, stderr) {
+  var diff  = (new Date()) - startSleep3;
+  // We should have already killed this process. Assert that
+  // the timeout still works and that we are getting the proper callback
+  // parameters.
+  assert.ok(err);
+  assert.ok(err.killed);
+  assert.equal(err.signal, 'SIGTERM');
+
+  // the timeout should still be in effect
+  console.log("'sleep 3' was already killed. Took %d ms", diff);
+  assert.ok(diff < 1500);
+}
+
+
+
 exec('python -c "print 200000*\'C\'"', { maxBuffer: 1000 }, function (err, stdout, stderr) {
   assert.ok(err);
   assert.ok(err.killed);
-  assert.equal(err.signal, 'SIGKILL');
+  assert.equal(err.signal, 'SIGTERM');
 });
 
 process.addListener("exit", function () {