This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

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:
bd8e4f6
5a98fa4
6570cd9
9bf2975
  • Loading branch information...
1 parent d3e6834 commit f7bc7fb0317a3417595ed9822fbdcb0bc2f26f61 @ry ry committed Oct 23, 2010
Showing with 89 additions and 31 deletions.
  1. +2 −2 doc/api.markdown
  2. +47 −26 lib/child_process.js
  3. +40 −3 test/simple/test-exec.js
View
@@ -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.
View
@@ -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);
+ }
};
View
@@ -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 () {

0 comments on commit f7bc7fb

Please sign in to comment.