Skip to content

Commit

Permalink
child_process: setup stdio on error when possible
Browse files Browse the repository at this point in the history
As more spawn() errors are classified as runtime errors, it's
no longer appropriate to only check UV_ENOENT when determining
if stdio can be setup. This commit reverses the check to look
for EMFILE and ENFILE specifically.

PR-URL: nodejs#27696
Fixes: nodejs#26852
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
cjihrig committed May 20, 2019
1 parent 9f71dbc commit 64182e9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
7 changes: 3 additions & 4 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,11 @@ ChildProcess.prototype.spawn = function(options) {
err === UV_ENFILE ||
err === UV_ENOENT) {
process.nextTick(onErrorNT, this, err);

// There is no point in continuing when we've hit EMFILE or ENFILE
// because we won't be able to set up the stdio file descriptors.
// It's kind of silly that the de facto spec for ENOENT (the test suite)
// mandates that stdio _is_ set up, even if there is no process on the
// receiving end, but it is what it is.
if (err !== UV_ENOENT) return err;
if (err === UV_EMFILE || err === UV_ENFILE)
return err;
} else if (err) {
// Close all opened fds on error
for (i = 0; i < stdio.length; i++) {
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-child-process-spawn-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ const spawnargs = ['bar'];
assert.strictEqual(fs.existsSync(enoentPath), false);

const enoentChild = spawn(enoentPath, spawnargs);

// Verify that stdio is setup if the error is not EMFILE or ENFILE.
assert.notStrictEqual(enoentChild.stdin, undefined);
assert.notStrictEqual(enoentChild.stdout, undefined);
assert.notStrictEqual(enoentChild.stderr, undefined);
assert(Array.isArray(enoentChild.stdio));
assert.strictEqual(enoentChild.stdio[0], enoentChild.stdin);
assert.strictEqual(enoentChild.stdio[1], enoentChild.stdout);
assert.strictEqual(enoentChild.stdio[2], enoentChild.stderr);

enoentChild.on('error', common.mustCall(function(err) {
assert.strictEqual(err.code, 'ENOENT');
assert.strictEqual(err.errno, 'ENOENT');
Expand Down
6 changes: 6 additions & 0 deletions test/sequential/test-child-process-emfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ for (;;) {
// Should emit an error, not throw.
const proc = child_process.spawn(process.execPath, ['-e', '0']);

// Verify that stdio is not setup on EMFILE or ENFILE.
assert.strictEqual(proc.stdin, undefined);
assert.strictEqual(proc.stdout, undefined);
assert.strictEqual(proc.stderr, undefined);
assert.strictEqual(proc.stdio, undefined);

proc.on('error', common.mustCall(function(err) {
assert.strictEqual(err.code, 'EMFILE');
}));
Expand Down

0 comments on commit 64182e9

Please sign in to comment.