summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorcjihrig <cjihrig@gmail.com>2019-05-14 12:38:14 -0400
committerRuben Bridgewater <ruben@bridgewater.de>2019-05-21 13:13:24 +0200
commit2bc177aa4fa686b99bcdbc05fd6500187085ec1d (patch)
treed6663ef52da1fce7c5b2bbb710083b2a907a5a7c
parent10b4a8103d5e8a081f9e6a6b8911d02c4e4e3dc1 (diff)
downloadnode-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.js7
-rw-r--r--test/parallel/test-child-process-spawn-error.js10
-rw-r--r--test/sequential/test-child-process-emfile.js6
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');
}));