Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
test: Worker initialization failure test case
Cover the scenario fixed through
#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.
  • Loading branch information
HarshithaKP committed Feb 24, 2020
commit f69276b3eaf3c55b9f7b17d89072d332b2046d3b
43 changes: 43 additions & 0 deletions test/parallel/test-worker-init-failed.js
Original file line number Diff line number Diff line change
@@ -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());
})
}
43 changes: 43 additions & 0 deletions test/parallel/test-worker-init-failure.js
Original file line number Diff line number Diff line change
@@ -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/));
Comment thread
Trott marked this conversation as resolved.
Resolved
assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED');
});
}
Comment thread
addaleax marked this conversation as resolved.
Resolved

} else {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@lundibundi, thanks. Removed the extra line.

if (common.isWindows) {
common.skip('ulimit does not work on Windows.');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe instead of skipping the test, just skip the ulimit prefix? As written, I don't think the test won't fail if the error event never happens, so the test might still be runnable under Windows?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't think the test won't fail if the error event never happens

I think this is an error in the test and therefore after it is resolved it will no longer be runnable on windows.
Also, it doesn't look like there is any equivalent for ulimit on windows so skip looks like the only way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Trott , with respect to Windows - is there any merit in running a test that doesn't make any assertion ?

}
Comment thread
lundibundi marked this conversation as resolved.
Resolved

// Limit the number of open files, to force workers to fail
let testCmd = 'ulimit -n 128 && ';
Comment thread
lundibundi marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any reason not to go with something much smaller than 128, like maybe 8?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Trott, When I ran a simple node.js code, it created 30+ file descriptors. If I reduce the fd count to 8, it fails at arbitrary point without / before reaching worker creation.


testCmd += `${process.argv[0]} ${process.argv[1]} child`;
Comment thread
lundibundi marked this conversation as resolved.
Resolved
const cp = child_process.exec(testCmd);
Comment thread
lundibundi marked this conversation as resolved.
Resolved

// Turn on the child streams for debugging purpose
Comment thread
lundibundi marked this conversation as resolved.
Resolved
cp.stderr.on('data', (d) => {
console.log(d.toString());
});
cp.stdout.on('data', (d) => {
console.log(d.toString());
});
Comment thread
lundibundi marked this conversation as resolved.
Resolved
}
Comment thread
lundibundi marked this conversation as resolved.
Resolved