From f69276b3eaf3c55b9f7b17d89072d332b2046d3b Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Mon, 24 Feb 2020 02:28:26 -0500 Subject: [PATCH 1/9] test: Worker initialization failure test case Cover the scenario fixed through https://github.com/nodejs/node/pull/31621 Unfortunately there is no easy way to test this, in a cross-platform manner. So the approach is: - open a child process with ulimit restriction on file descriptors - in the child process, start few workers - more than the fd limit - make sure some workers fail, with the expected error type. - skip the test in windows, as there is no ulimit there. --- test/parallel/test-worker-init-failed.js | 43 +++++++++++++++++++++++ test/parallel/test-worker-init-failure.js | 43 +++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 test/parallel/test-worker-init-failed.js create mode 100644 test/parallel/test-worker-init-failure.js diff --git a/test/parallel/test-worker-init-failed.js b/test/parallel/test-worker-init-failed.js new file mode 100644 index 00000000000000..c19476e33cde1a --- /dev/null +++ b/test/parallel/test-worker-init-failed.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); + +// Test that workers fail with meaningful error message +// when their initialization fails. + +if (process.argv[2] === 'child') { + const {Worker} = require('worker_threads'); + for (let i = 0; i < 256; ++i) { + const worker = new Worker( + 'require(\'worker_threads\').parentPort.postMessage(2 + 2)', + { eval: true }); + worker.on('message', (result) => { + assert.strictEqual(result, 4); + }) + worker.on('error', (e) => { + assert(e.message.match(/Worker initialization failure: EMFILE/)); + assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED'); + }) + } + +} else { + + if (common.isWindows) { + common.skip('ulimit does not work on Windows.'); + } + + // limit the number of open files, to force workers to fail + let testCmd = 'ulimit -n 128 && '; + + testCmd += `${process.argv[0]} ${process.argv[1]} child`; + const cp = child_process.exec(testCmd); + + // turn on the child streams for debugging purpose + cp.stderr.on('data', (d) => { + console.log(d.toString()); + }) + cp.stdout.on('data', (d) => { + console.log(d.toString()); + }) +} diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js new file mode 100644 index 00000000000000..690db75e455c55 --- /dev/null +++ b/test/parallel/test-worker-init-failure.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); + +// Test that workers fail with meaningful error message +// when their initialization fails. + +if (process.argv[2] === 'child') { + const { Worker } = require('worker_threads'); + for (let i = 0; i < 256; ++i) { + const worker = new Worker( + 'require(\'worker_threads\').parentPort.postMessage(2 + 2)', + { eval: true }); + worker.on('message', (result) => { + assert.strictEqual(result, 4); + }); + worker.on('error', (e) => { + assert(e.message.match(/Worker initialization failure: EMFILE/)); + assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED'); + }); + } + +} else { + + if (common.isWindows) { + common.skip('ulimit does not work on Windows.'); + } + + // Limit the number of open files, to force workers to fail + let testCmd = 'ulimit -n 128 && '; + + testCmd += `${process.argv[0]} ${process.argv[1]} child`; + const cp = child_process.exec(testCmd); + + // Turn on the child streams for debugging purpose + cp.stderr.on('data', (d) => { + console.log(d.toString()); + }); + cp.stdout.on('data', (d) => { + console.log(d.toString()); + }); +} From 500e991896aec61ebce40a69877de78239fdad27 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Mon, 24 Feb 2020 02:46:12 -0500 Subject: [PATCH 2/9] fixup: remove unwanted file --- test/parallel/test-worker-init-failed.js | 43 ------------------------ 1 file changed, 43 deletions(-) delete mode 100644 test/parallel/test-worker-init-failed.js diff --git a/test/parallel/test-worker-init-failed.js b/test/parallel/test-worker-init-failed.js deleted file mode 100644 index c19476e33cde1a..00000000000000 --- a/test/parallel/test-worker-init-failed.js +++ /dev/null @@ -1,43 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const child_process = require('child_process'); - -// Test that workers fail with meaningful error message -// when their initialization fails. - -if (process.argv[2] === 'child') { - const {Worker} = require('worker_threads'); - for (let i = 0; i < 256; ++i) { - const worker = new Worker( - 'require(\'worker_threads\').parentPort.postMessage(2 + 2)', - { eval: true }); - worker.on('message', (result) => { - assert.strictEqual(result, 4); - }) - worker.on('error', (e) => { - assert(e.message.match(/Worker initialization failure: EMFILE/)); - assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED'); - }) - } - -} else { - - if (common.isWindows) { - common.skip('ulimit does not work on Windows.'); - } - - // limit the number of open files, to force workers to fail - let testCmd = 'ulimit -n 128 && '; - - testCmd += `${process.argv[0]} ${process.argv[1]} child`; - const cp = child_process.exec(testCmd); - - // turn on the child streams for debugging purpose - cp.stderr.on('data', (d) => { - console.log(d.toString()); - }) - cp.stdout.on('data', (d) => { - console.log(d.toString()); - }) -} From 313d520f63cca47f2d5172d228f7125e85ff4cd1 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Tue, 25 Feb 2020 06:35:11 -0500 Subject: [PATCH 3/9] fixup: address review comments --- test/parallel/test-worker-init-failure.js | 46 ++++++++++++++++------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 690db75e455c55..29d8ad0a0750e8 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -6,9 +6,22 @@ const child_process = require('child_process'); // Test that workers fail with meaningful error message // when their initialization fails. +if (common.isWindows) { + common.skip('ulimit does not work on Windows.'); +} + +// A reasonably low fd count. An empty node process +// creates around 30 fds for its internal purposes, +// so making it too low will crash the process early, +// making it too high will cause too much resource use. +const OPENFILES = 128; + +// Double the open files - so that some workers fail for sure. +const WORKERCOUNT = 256; + if (process.argv[2] === 'child') { const { Worker } = require('worker_threads'); - for (let i = 0; i < 256; ++i) { + for (let i = 0; i < WORKERCOUNT; ++i) { const worker = new Worker( 'require(\'worker_threads\').parentPort.postMessage(2 + 2)', { eval: true }); @@ -16,28 +29,35 @@ if (process.argv[2] === 'child') { assert.strictEqual(result, 4); }); worker.on('error', (e) => { - assert(e.message.match(/Worker initialization failure: EMFILE/)); + assert.match(e.message, /Worker initialization failure: EMFILE/); assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED'); }); } } else { - - if (common.isWindows) { - common.skip('ulimit does not work on Windows.'); - } - // Limit the number of open files, to force workers to fail - let testCmd = 'ulimit -n 128 && '; + let testCmd = `ulimit -n ${OPENFILES} && `; - testCmd += `${process.argv[0]} ${process.argv[1]} child`; + testCmd += `${process.execPath} ${__filename} child`; const cp = child_process.exec(testCmd); + let stdout = ''; + let stderr = ''; + + cp.on('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + if (stdout !== '') + console.log(`child stdout: ${stdout}`); + if (stderr !== '') + console.log(`child stderr: ${stderr}`); + })); + // Turn on the child streams for debugging purpose - cp.stderr.on('data', (d) => { - console.log(d.toString()); + cp.stderr.on('data', (chunk) => { + stdout += chunk; }); - cp.stdout.on('data', (d) => { - console.log(d.toString()); + cp.stdout.on('data', (chunk) => { + stderr += chunk; }); } From 44042cae1fe27e2c4816fd1831e7924740229eb2 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Wed, 26 Feb 2020 01:15:10 -0500 Subject: [PATCH 4/9] fixup: address review comments --- test/parallel/test-worker-init-failure.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 29d8ad0a0750e8..662ea2e54f8400 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -35,7 +35,7 @@ if (process.argv[2] === 'child') { } } else { - // Limit the number of open files, to force workers to fail + // Limit the number of open files, to force workers to fail. let testCmd = `ulimit -n ${OPENFILES} && `; testCmd += `${process.execPath} ${__filename} child`; @@ -45,15 +45,15 @@ if (process.argv[2] === 'child') { let stderr = ''; cp.on('exit', common.mustCall((code, signal) => { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); if (stdout !== '') console.log(`child stdout: ${stdout}`); if (stderr !== '') console.log(`child stderr: ${stderr}`); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); })); - // Turn on the child streams for debugging purpose + // Turn on the child streams for debugging purposes. cp.stderr.on('data', (chunk) => { stdout += chunk; }); From 6a7f905382ea674dc0a29cf6819e186cf20bab63 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Wed, 26 Feb 2020 02:35:44 -0500 Subject: [PATCH 5/9] fixup: cover EMFILE failures generically --- test/parallel/test-worker-init-failure.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 662ea2e54f8400..932a9f39d79279 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -29,8 +29,8 @@ if (process.argv[2] === 'child') { assert.strictEqual(result, 4); }); worker.on('error', (e) => { - assert.match(e.message, /Worker initialization failure: EMFILE/); - assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED'); + assert.match(e.message, /EMFILE/); + assert.ok(e.code === 'ERR_WORKER_INIT_FAILED' || e.code === 'EMFILE'); }); } From 6a6efb9cbcc1c1cc37182d0c6c8c7531cec7c668 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Thu, 27 Feb 2020 04:41:25 -0500 Subject: [PATCH 6/9] fixup: address review comment --- test/parallel/test-worker-init-failure.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 932a9f39d79279..e0b17326f3291b 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -41,23 +41,20 @@ if (process.argv[2] === 'child') { testCmd += `${process.execPath} ${__filename} child`; const cp = child_process.exec(testCmd); + // Turn on the child streams for debugging purposes. let stdout = ''; + cp.stdout.on('data', (chunk) => { + stdout += chunk; + }); let stderr = ''; + cp.stderr.on('data', (chunk) => { + stderr += chunk; + }); cp.on('exit', common.mustCall((code, signal) => { - if (stdout !== '') - console.log(`child stdout: ${stdout}`); - if (stderr !== '') - console.log(`child stderr: ${stderr}`); + console.log(`child stdout: ${stdout}\n`); + console.log(`child stderr: ${stderr}\n`); assert.strictEqual(code, 0); assert.strictEqual(signal, null); })); - - // Turn on the child streams for debugging purposes. - cp.stderr.on('data', (chunk) => { - stdout += chunk; - }); - cp.stdout.on('data', (chunk) => { - stderr += chunk; - }); } From d356e7e1c9bbfcd7b40208a31718f19343a318f3 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Thu, 27 Feb 2020 05:49:37 -0500 Subject: [PATCH 7/9] fixup: address review comment --- test/parallel/test-worker-init-failure.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index e0b17326f3291b..7cd19ffc37fe2e 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -28,6 +28,12 @@ if (process.argv[2] === 'child') { worker.on('message', (result) => { assert.strictEqual(result, 4); }); + + // We want to test that if there is an error in a constrained running + // environment, it will be one of `EMFILE` or `ERR_WORKER_INIT_FAILED`. + // `common.mustCall*` cannot be used here as in some environments + // (i.e. single cpu) `ulimit` may not lead to such an error. + worker.on('error', (e) => { assert.match(e.message, /EMFILE/); assert.ok(e.code === 'ERR_WORKER_INIT_FAILED' || e.code === 'EMFILE'); From 520a906cebfbdebcb2afc25fc9beb0cae7d6a841 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Wed, 11 Mar 2020 08:58:57 -0400 Subject: [PATCH 8/9] fixup: address review comments --- test/parallel/test-worker-init-failure.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 7cd19ffc37fe2e..935c6d63b5ef09 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -43,16 +43,17 @@ if (process.argv[2] === 'child') { } else { // Limit the number of open files, to force workers to fail. let testCmd = `ulimit -n ${OPENFILES} && `; - testCmd += `${process.execPath} ${__filename} child`; const cp = child_process.exec(testCmd); // Turn on the child streams for debugging purposes. let stdout = ''; + cp.stdout.setEncoding('utf8'); cp.stdout.on('data', (chunk) => { stdout += chunk; }); let stderr = ''; + cp.stdout.setEncoding('utf8'); cp.stderr.on('data', (chunk) => { stderr += chunk; }); From 1c804df7369fb9f9ce26a15984dd8e3cac8cd9fd Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Wed, 11 Mar 2020 09:21:43 -0400 Subject: [PATCH 9/9] fixup: address review comment --- test/parallel/test-worker-init-failure.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-worker-init-failure.js b/test/parallel/test-worker-init-failure.js index 935c6d63b5ef09..ea9654200ecaf8 100644 --- a/test/parallel/test-worker-init-failure.js +++ b/test/parallel/test-worker-init-failure.js @@ -53,7 +53,7 @@ if (process.argv[2] === 'child') { stdout += chunk; }); let stderr = ''; - cp.stdout.setEncoding('utf8'); + cp.stderr.setEncoding('utf8'); cp.stderr.on('data', (chunk) => { stderr += chunk; });