From 6570cd99e51adb082d0e0076a1205cd867f4ce41 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Wed, 20 Oct 2010 19:20:44 -0700
Subject: [PATCH] Fix execFile timeouts, improve tests

It seems that a parent will not get a SIGCHLD if the child is killed by the
parent? It's unclear, so make 'exit' callback manually.
---
 lib/child_process.js     | 39 +++++++++++++++++++++++++--------------
 test/simple/test-exec.js | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/lib/child_process.js b/lib/child_process.js
index 78d9a96250..e0d816a740 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -61,10 +61,31 @@ exports.execFile = function (file /* args, options, callback */) {
   var stdout = "";
   var stderr = "";
   var killed = false;
+  var exited = false;
+
+  function exithandler (code, signal) {
+    if (timeoutId) clearTimeout(timeoutId);
+    if (exited) return;
+    exited = true;
+    if (!callback) return;
+
+    if (code === 0 && signal === null) {
+      callback(null, stdout, stderr);
+    } else {
+      var e = new Error("Command failed: " + stderr);
+      e.killed = killed;
+      e.code = code;
+      e.signal = signal;
+      callback(e, stdout, stderr);
+    }
+  }
 
   function kill () {
-    child.kill(options.killSignal);
+    var c = child.kill(options.killSignal);
     killed = true;
+    process.nextTick(function () {
+      exithandler(null, options.killSignal)
+    });
   }
 
   var timeoutId;
@@ -92,19 +113,9 @@ exports.execFile = function (file /* args, options, callback */) {
     }
   });
 
-  child.addListener("exit", function (code, signal) {
-    if (timeoutId) clearTimeout(timeoutId);
-    if (!callback) return;
-    if (code === 0 && signal === null) {
-      callback(null, stdout, stderr);
-    } else {
-      var e = new Error("Command failed: " + stderr);
-      e.killed = killed;
-      e.code = code;
-      e.signal = signal;
-      callback(e, stdout, stderr);
-    }
-  });
+  var pid = child.pid;
+
+  child.addListener("exit", exithandler);
 
   return child;
 };
diff --git a/test/simple/test-exec.js b/test/simple/test-exec.js
index 8cfc46f3e3..8819bd1262 100644
--- a/test/simple/test-exec.js
+++ b/test/simple/test-exec.js
@@ -35,22 +35,48 @@ exec("ls /DOES_NOT_EXIST", 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');
 });
 
 
-var killMeTwice = exec("sleep 3", { timeout: 1000 }, function killMeTwiceCallback(err, stdout, stderr) {
-  assert.ok(err);
-  assert.ok(err.killed);
-  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, 'SIGKILL');
+
+  // 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);