Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Worker initialization failure test case #31929

Closed
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/));
Trott marked this conversation as resolved.
Show resolved Hide resolved
assert.strictEqual(e.code, 'ERR_WORKER_INIT_FAILED');
});
}
addaleax marked this conversation as resolved.
Show resolved Hide resolved

} else {

Copy link
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
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
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
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
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 ?

}
lundibundi marked this conversation as resolved.
Show resolved Hide resolved

// Limit the number of open files, to force workers to fail
let testCmd = 'ulimit -n 128 && ';
lundibundi marked this conversation as resolved.
Show resolved Hide resolved
Copy link
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
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`;
lundibundi marked this conversation as resolved.
Show resolved Hide resolved
const cp = child_process.exec(testCmd);
lundibundi marked this conversation as resolved.
Show resolved Hide resolved

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