diff options
author | cjihrig <cjihrig@gmail.com> | 2019-05-14 12:38:14 -0400 |
---|---|---|
committer | Ruben Bridgewater <ruben@bridgewater.de> | 2019-05-21 13:13:24 +0200 |
commit | 2bc177aa4fa686b99bcdbc05fd6500187085ec1d (patch) | |
tree | d6663ef52da1fce7c5b2bbb710083b2a907a5a7c | |
parent | 10b4a8103d5e8a081f9e6a6b8911d02c4e4e3dc1 (diff) | |
download | node-new-2bc177aa4fa686b99bcdbc05fd6500187085ec1d.tar.gz |
child_process: setup stdio on error when possible
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: https://github.com/nodejs/node/pull/27696
Fixes: https://github.com/nodejs/node/issues/26852
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
-rw-r--r-- | lib/internal/child_process.js | 7 | ||||
-rw-r--r-- | test/parallel/test-child-process-spawn-error.js | 10 | ||||
-rw-r--r-- | test/sequential/test-child-process-emfile.js | 6 |
3 files changed, 19 insertions, 4 deletions
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 4e6a25a8a3..a0f083d8de 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -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++) { diff --git a/test/parallel/test-child-process-spawn-error.js b/test/parallel/test-child-process-spawn-error.js index 28591f88fa..33f66d0ba6 100644 --- a/test/parallel/test-child-process-spawn-error.js +++ b/test/parallel/test-child-process-spawn-error.js @@ -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'); diff --git a/test/sequential/test-child-process-emfile.js b/test/sequential/test-child-process-emfile.js index 7e31792f9d..8091e54a5c 100644 --- a/test/sequential/test-child-process-emfile.js +++ b/test/sequential/test-child-process-emfile.js @@ -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'); })); |