From c7b8073afc018901a106564aaf6819b170a9538a Mon Sep 17 00:00:00 2001 From: Charlie McConnell Date: Tue, 13 Mar 2012 16:04:00 -0700 Subject: [PATCH] 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. --- lib/child_process.js | 14 ++++++++------ test/simple/test-child-process-buffering.js | 9 +++++++++ test/simple/test-child-process-cwd.js | 5 ++++- test/simple/test-child-process-stdin.js | 6 ++++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 3355b8ab28c..0a0ae920528 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -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); } } @@ -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,7 +476,7 @@ ChildProcess.prototype.spawn = function(options) { this.stdout = createSocket(options.stdoutStream, true); this._closesNeeded++; this.stdout.on('close', function() { - maybeExit(self); + maybeClose(self); }); } @@ -482,7 +484,7 @@ ChildProcess.prototype.spawn = function(options) { this.stderr = createSocket(options.stderrStream, true); this._closesNeeded++; this.stderr.on('close', function() { - maybeExit(self); + maybeClose(self); }); } diff --git a/test/simple/test-child-process-buffering.js b/test/simple/test-child-process-buffering.js index 225e3cc3aaf..3fb13b6208f 100644 --- a/test/simple/test-child-process-buffering.js +++ b/test/simple/test-child-process-buffering.js @@ -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); }); diff --git a/test/simple/test-child-process-cwd.js b/test/simple/test-child-process-cwd.js index 37195ab5d49..dc8283b7a96 100644 --- a/test/simple/test-child-process-cwd.js +++ b/test/simple/test-child-process-cwd.js @@ -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--; }); diff --git a/test/simple/test-child-process-stdin.js b/test/simple/test-child-process-stdin.js index a58b0629264..fdb40cc82c4 100644 --- a/test/simple/test-child-process-stdin.js +++ b/test/simple/test-child-process-stdin.js @@ -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 {