Skip to content

Commit

Permalink
child_process: Separate 'close' event from 'exit'
Browse files Browse the repository at this point in the history
Currently, a child process does not emit the 'exit' event until 'close' events
have been received on all three of the child's stdio streams.  This change makes
the child object emit 'exit' when the child exits, and a new 'close' event when
all stdio streams are closed.
  • Loading branch information
AvianFlu authored and isaacs committed Mar 16, 2012
1 parent 928ea56 commit c7b8073
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 7 deletions.
14 changes: 8 additions & 6 deletions lib/child_process.js
Expand Up @@ -341,7 +341,7 @@ exports.execFile = function(file /* args, options, callback */) {
}
});

child.addListener('exit', exithandler);
child.addListener('close', exithandler);

return child;
};
Expand Down Expand Up @@ -373,11 +373,11 @@ var spawn = exports.spawn = function(file, args, options) {
};


function maybeExit(subprocess) {
function maybeClose(subprocess) {
subprocess._closesGot++;

if (subprocess._closesGot == subprocess._closesNeeded) {
subprocess.emit('exit', subprocess.exitCode, subprocess.signalCode);
subprocess.emit('close', subprocess.exitCode, subprocess.signalCode);
}
}

Expand Down Expand Up @@ -413,7 +413,9 @@ function ChildProcess() {
self._internal.close();
self._internal = null;

maybeExit(self);
self.emit('exit', self.exitCode, self.signalCode);

maybeClose(self);
};
}
util.inherits(ChildProcess, EventEmitter);
Expand Down Expand Up @@ -474,15 +476,15 @@ ChildProcess.prototype.spawn = function(options) {
this.stdout = createSocket(options.stdoutStream, true);
this._closesNeeded++;
this.stdout.on('close', function() {
maybeExit(self);
maybeClose(self);
});
}

if (options.stderrStream) {
this.stderr = createSocket(options.stderrStream, true);
this._closesNeeded++;
this.stderr.on('close', function() {
maybeExit(self);
maybeClose(self);
});
}

Expand Down
9 changes: 9 additions & 0 deletions test/simple/test-child-process-buffering.js
Expand Up @@ -25,6 +25,8 @@ var assert = require('assert');
var spawn = require('child_process').spawn;

var pwd_called = false;
var childClosed = false;
var childExited = false;

function pwd(callback) {
var output = '';
Expand All @@ -39,8 +41,13 @@ function pwd(callback) {
child.on('exit', function(c) {
console.log('exit: ' + c);
assert.equal(0, c);
childExited = true;
});

child.on('close', function () {
callback(output);
pwd_called = true;
childClosed = true;
});
}

Expand All @@ -53,4 +60,6 @@ pwd(function(result) {

process.on('exit', function() {
assert.equal(true, pwd_called);
assert.equal(true, childExited);
assert.equal(true, childClosed);
});
5 changes: 4 additions & 1 deletion test/simple/test-child-process-cwd.js
Expand Up @@ -44,8 +44,11 @@ function testCwd(options, forCode, forData) {
});

child.on('exit', function(code, signal) {
forData && assert.strictEqual(forData, data.replace(/[\s\r\n]+$/, ''));
assert.strictEqual(forCode, code);
});

child.on('close', function () {
forData && assert.strictEqual(forData, data.replace(/[\s\r\n]+$/, ''));
returns--;
});

Expand Down
6 changes: 6 additions & 0 deletions test/simple/test-child-process-stdin.js
Expand Up @@ -37,6 +37,7 @@ cat.stdin.end();

var response = '';
var exitStatus = -1;
var closed = false;

var gotStdoutEOF = false;

Expand Down Expand Up @@ -66,6 +67,10 @@ cat.stderr.on('end', function(chunk) {
cat.on('exit', function(status) {
console.log('exit event');
exitStatus = status;
});

cat.on('close', function () {
closed = true;
if (is_windows) {
assert.equal('hello world\r\n', response);
} else {
Expand All @@ -75,6 +80,7 @@ cat.on('exit', function(status) {

process.on('exit', function() {
assert.equal(0, exitStatus);
assert(closed);
if (is_windows) {
assert.equal('hello world\r\n', response);
} else {
Expand Down

0 comments on commit c7b8073

Please sign in to comment.