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

worker: add args constructor option #30559

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@ changes:
- version: v13.2.0
pr-url: https://github.com/nodejs/node/pull/26628
description: The `resourceLimits` option was introduced.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30559
description: The `argv` option was introduced.
-->

* `filename` {string} The path to the Worker’s main script. Must be
Expand All @@ -518,6 +521,10 @@ changes:
If `options.eval` is `true`, this is a string containing JavaScript code
rather than a path.
* `options` {Object}
* `argv` {any[]} List of arguments which would be stringified and appended to
`process.argv` in the worker. This is mostly similar to the `workerData`
but the values will be available on the global `process.argv` as if they
were passed as CLI options to the script.
* `env` {Object} If set, specifies the initial value of `process.env` inside
the Worker thread. As a special value, [`worker.SHARE_ENV`][] may be used
to specify that the parent thread and the child thread should share their
Expand Down
18 changes: 16 additions & 2 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ if (process.env.NODE_CHANNEL_FD) {
port.on('message', (message) => {
if (message.type === LOAD_SCRIPT) {
const {
argv,
cwdCounter,
filename,
doEval,
Expand All @@ -115,6 +116,9 @@ port.on('message', (message) => {
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
loadPreloadModules();
initializeFrozenIntrinsics();
if (argv !== undefined) {
process.argv = process.argv.concat(argv);
}
publicWorker.parentPort = publicPort;
publicWorker.workerData = workerData;

Expand Down Expand Up @@ -143,12 +147,22 @@ port.on('message', (message) => {
port.postMessage({ type: UP_AND_RUNNING });
if (doEval) {
const { evalScript } = require('internal/process/execution');
evalScript('[worker eval]', filename);
const name = '[worker eval]';
// This is necessary for CJS module compilation.
// TODO: pass this with something really internal.
ObjectDefineProperty(process, '_eval', {
Copy link
Member

Choose a reason for hiding this comment

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

Would a symbol be better/possible here?

Copy link
Member

Choose a reason for hiding this comment

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

This is to match with what the CJS loader expects for the --eval CLI option, unfortunately

configurable: true,
enumerable: true,
value: filename,
});
process.argv.splice(1, 0, name);
evalScript(name, filename);
} else {
// script filename
// runMain here might be monkey-patched by users in --require.
// XXX: the monkey-patchability here should probably be deprecated.
CJSLoader.Module.runMain(process.argv[1] = filename);
process.argv.splice(1, 0, filename);
CJSLoader.Module.runMain(filename);
}
} else if (message.type === STDIO_PAYLOAD) {
const { stream, chunk, encoding } = message;
Expand Down
10 changes: 9 additions & 1 deletion lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,16 @@ class Worker extends EventEmitter {
validateString(filename, 'filename');
if (options.execArgv && !ArrayIsArray(options.execArgv)) {
throw new ERR_INVALID_ARG_TYPE('options.execArgv',
'array',
'Array',
options.execArgv);
}
let argv;
if (options.argv) {
if (!ArrayIsArray(options.argv)) {
throw new ERR_INVALID_ARG_TYPE('options.argv', 'Array', options.argv);
}
argv = options.argv.map(String);
}
if (!options.eval) {
if (!path.isAbsolute(filename) && !/^\.\.?[\\/]/.test(filename)) {
throw new ERR_WORKER_PATH(filename);
Expand Down Expand Up @@ -154,6 +161,7 @@ class Worker extends EventEmitter {
this[kPublicPort].on('message', (message) => this.emit('message', message));
setupPortReferencing(this[kPublicPort], this, 'message');
this[kPort].postMessage({
argv,
type: messageTypes.LOAD_SCRIPT,
filename,
doEval: !!options.eval,
Expand Down
49 changes: 49 additions & 0 deletions test/parallel/test-worker-process-argv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker, isMainThread, workerData } = require('worker_threads');

if (isMainThread) {
assert.throws(() => {
new Worker(__filename, { argv: 'foo' });
}, {
code: 'ERR_INVALID_ARG_TYPE'
});

[
new Worker(__filename, {
argv: [null, 'foo', 123, Symbol('bar')],
// Asserts only if the worker is started by the test.
workerData: 'assert-argv'
}),
new Worker(`
const assert = require('assert');
assert.deepStrictEqual(
process.argv,
[process.execPath, '[worker eval]']
);
`, {
eval: true
}),
new Worker(`
const assert = require('assert');
assert.deepStrictEqual(
process.argv,
[process.execPath, '[worker eval]', 'null', 'foo', '123',
String(Symbol('bar'))]
);
`, {
argv: [null, 'foo', 123, Symbol('bar')],
eval: true
})
].forEach((worker) => {
worker.on('exit', common.mustCall((code) => {
assert.strictEqual(code, 0);
}));
});
} else if (workerData === 'assert-argv') {
assert.deepStrictEqual(
process.argv,
[process.execPath, __filename, 'null', 'foo', '123', String(Symbol('bar'))]
);
}