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

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 13, 2012
1 parent 928ea56 commit c7b8073afc018901a106564aaf6819b170a9538a
View
@@ -341,7 +341,7 @@ exports.execFile = function(file /* args, options, callback */) {
}
});
- child.addListener('exit', exithandler);
+ child.addListener('close', exithandler);
return child;
};
@@ -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.

Show comment Hide comment
@igorzi

igorzi Mar 20, 2012

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

@igorzi

igorzi Mar 20, 2012

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

This comment has been minimized.

Show comment Hide comment
@isaacs

isaacs Mar 20, 2012

No, but it doesn't hurt.

@isaacs

isaacs Mar 20, 2012

No, but it doesn't hurt.

}
}
@@ -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);
@@ -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);
});
}
@@ -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 = '';
@@ -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;
});
}
@@ -53,4 +60,6 @@ pwd(function(result) {
process.on('exit', function() {
assert.equal(true, pwd_called);
+ assert.equal(true, childExited);
+ assert.equal(true, childClosed);
});
@@ -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--;
});
@@ -37,6 +37,7 @@ cat.stdin.end();
var response = '';
var exitStatus = -1;
+var closed = false;
var gotStdoutEOF = false;
@@ -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 {
@@ -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 {

0 comments on commit c7b8073

Please sign in to comment.