Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child_process: refactor internal/child_process.js #11366

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,16 @@ util.inherits(ChildProcess, EventEmitter);


function flushStdio(subprocess) {
if (subprocess.stdio == null) return;
subprocess.stdio.forEach(function(stream, fd, stdio) {
const stdio = subprocess.stdio;

if (stdio == null) return;

for (var i = 0; i < stdio.length; i++) {
const stream = stdio[i];
if (!stream || !stream.readable || stream._readableState.readableListening)
return;
continue;
stream.resume();
});
}
}


Expand Down Expand Up @@ -268,6 +272,7 @@ ChildProcess.prototype.spawn = function(options) {
const self = this;
var ipc;
var ipcFd;
var i;
// If no `stdio` option was given - use default
var stdio = options.stdio || 'pipe';

Expand Down Expand Up @@ -302,11 +307,12 @@ ChildProcess.prototype.spawn = function(options) {
if (err !== uv.UV_ENOENT) return err;
} else if (err) {
// Close all opened fds on error
stdio.forEach(function(stdio) {
if (stdio.type === 'pipe') {
stdio.handle.close();
for (i = 0; i < stdio.length; i++) {
const stream = stdio[i];
if (stream.type === 'pipe') {
stream.handle.close();
}
});
}

this._handle.close();
this._handle = null;
Expand All @@ -315,27 +321,29 @@ ChildProcess.prototype.spawn = function(options) {

this.pid = this._handle.pid;

stdio.forEach(function(stdio, i) {
if (stdio.type === 'ignore') return;
for (i = 0; i < stdio.length; i++) {
const stream = stdio[i];
if (stream.type === 'ignore') continue;

if (stdio.ipc) {
if (stream.ipc) {
self._closesNeeded++;
return;
continue;
}

if (stdio.handle) {
if (stream.handle) {
// when i === 0 - we're dealing with stdin
// (which is the only one writable pipe)
stdio.socket = createSocket(self.pid !== 0 ? stdio.handle : null, i > 0);
stream.socket = createSocket(self.pid !== 0 ?
stream.handle : null, i > 0);

if (i > 0 && self.pid !== 0) {
self._closesNeeded++;
stdio.socket.on('close', function() {
stream.socket.on('close', function() {
maybeClose(self);
});
}
}
});
}

this.stdin = stdio.length >= 1 && stdio[0].socket !== undefined ?
stdio[0].socket : null;
Expand Down Expand Up @@ -796,11 +804,11 @@ function _validateStdio(stdio, sync) {
}

// Defaults
if (stdio === null || stdio === undefined) {
if (stdio == null) {
stdio = i < 3 ? 'pipe' : 'ignore';
}

if (stdio === null || stdio === 'ignore') {
if (stdio === 'ignore') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? This would change behavior...

Copy link
Contributor Author

@notarseniy notarseniy Feb 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In previous condition we're handling stdio === null, so this condition never will be executed with stdio === null

acc.push({type: 'ignore'});
} else if (stdio === 'pipe' || typeof stdio === 'number' && stdio < 0) {
var a = {
Expand Down Expand Up @@ -887,7 +895,7 @@ function getSocketList(type, slave, key) {
function maybeClose(subprocess) {
subprocess._closesGot++;

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