Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
child_process: Separate 'close' event from 'exit'
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);

This comment has been minimized.

Copy link
@igorzi

igorzi Mar 20, 2012

Is it necessary for 'close' event to include exitCode & signalCode?

This comment has been minimized.

Copy link
@isaacs

isaacs Mar 20, 2012

No, but it doesn't hurt.

}
}

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.