Skip to content

Commit

Permalink
worker_threads: add type option to new Worker()
Browse files Browse the repository at this point in the history
Allows eval of ESM string.

Fixes: nodejs#30682
  • Loading branch information
aduh95 committed May 29, 2020
1 parent 108a1ab commit 4ed31e6
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 17 deletions.
16 changes: 10 additions & 6 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,14 +561,15 @@ if (isMainThread) {
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/31760
description: The `type` option was introduced.
- version:
- v13.13.0
- v12.17.0
pr-url: https://github.com/nodejs/node/pull/32278
description: The `transferList` option was introduced.
- version:
- v13.12.0
- v12.17.0
- version: v13.12.0
pr-url: https://github.com/nodejs/node/pull/31664
description: The `filename` parameter can be a WHATWG `URL` object using
`file:` protocol.
Expand Down Expand Up @@ -600,9 +601,12 @@ changes:
to specify that the parent thread and the child thread should share their
environment variables; in that case, changes to one thread’s `process.env`
object will affect the other thread as well. **Default:** `process.env`.
* `eval` {boolean} If `true` and the first argument is a `string`, interpret
the first argument to the constructor as a script that is executed once the
worker is online.
* `eval` {boolean} If `true`, interpret the first argument to the constructor
as a script (or a module if `type` is set to `module`) that is executed once
the worker is online.
* `type` {string} If `"module"` and `eval` is set to `true`, interpret the
first argument as an ECMAScript 2015 module instead of a script. The default
value is `"classic"`. Doesn't have any effect when `eval` is set to `false`.
* `execArgv` {string[]} List of node CLI options passed to the worker.
V8 options (such as `--max-old-space-size`) and options that affect the
process (such as `--title`) are not supported. If set, this will be provided
Expand Down
31 changes: 20 additions & 11 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// message port.

const {
ArrayPrototypeSplice,
ObjectDefineProperty,
} = primordials;

Expand Down Expand Up @@ -98,6 +99,7 @@ port.on('message', (message) => {
cwdCounter,
filename,
doEval,
workerType,
workerData,
publicPort,
manifestSrc,
Expand Down Expand Up @@ -148,17 +150,24 @@ port.on('message', (message) => {
`(eval = ${eval}) at cwd = ${process.cwd()}`);
port.postMessage({ type: UP_AND_RUNNING });
if (doEval) {
const { evalScript } = require('internal/process/execution');
const name = '[worker eval]';
// This is necessary for CJS module compilation.
// TODO: pass this with something really internal.
ObjectDefineProperty(process, '_eval', {
configurable: true,
enumerable: true,
value: filename,
});
process.argv.splice(1, 0, name);
evalScript(name, filename);
if (workerType === 'module') {
const { evalModule } = require('internal/process/execution');
evalModule(filename).catch((e) => {
workerOnGlobalUncaughtException(e, true);
});
} else {
const { evalScript } = require('internal/process/execution');
const name = '[worker eval]';
// This is necessary for CJS module compilation.
// TODO: pass this with something really internal.
ObjectDefineProperty(process, '_eval', {
configurable: true,
enumerable: true,
value: filename,
});
ArrayPrototypeSplice(process.argv, 1, 0, name);
evalScript(name, filename);
}
} else {
// script filename
// runMain here might be monkey-patched by users in --require.
Expand Down
12 changes: 12 additions & 0 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const {
} = workerIo;
const { deserializeError } = require('internal/error-serdes');
const { fileURLToPath, isURLInstance, pathToFileURL } = require('internal/url');
const { validateString } = require('internal/validators');

const {
ownsProcessState,
Expand Down Expand Up @@ -130,6 +131,16 @@ class Worker extends EventEmitter {
throw new ERR_WORKER_UNSUPPORTED_EXTENSION(ext);
}
}
if (options.type) {
validateString(options.type, 'options.type');
if (options.type !== 'module' && options.type !== 'classic') {
throw new ERR_INVALID_ARG_VALUE(
'options.type',
options.type,
'must be either "module" or "classic"'
);
}
}

let env;
if (typeof options.env === 'object' && options.env !== null) {
Expand Down Expand Up @@ -197,6 +208,7 @@ class Worker extends EventEmitter {
type: messageTypes.LOAD_SCRIPT,
filename,
doEval: !!options.eval,
workerType: options.type,
cwdCounter: cwdCounter || workerIo.sharedCwdCounter,
workerData: options.workerData,
publicPort: port2,
Expand Down
38 changes: 38 additions & 0 deletions test/parallel/test-worker-eval.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';
require('../common');
const assert = require('assert');
const { Worker } = require('worker_threads');

const moduleString = 'export default import.meta.url;';
const cjsString = 'module.exports=__filename;';

const cjsLoadingESMError = /Unexpected token 'export'/;
const esmLoadingCJSError = /module is not defined/;
const invalidValueError =
/The argument 'options\.type' must be either "module" or "classic"/;

function runInWorker(evalString, options) {
return new Promise((resolve, reject) => {
const worker = new Worker(evalString, options);
worker.on('error', reject);
worker.on('exit', resolve);
});
}

let options;

options = { eval: true };
assert.rejects(runInWorker(moduleString, options), cjsLoadingESMError);
runInWorker(cjsString, options); // does not reject

options = { eval: true, type: 'classic' };
assert.rejects(runInWorker(moduleString, options), cjsLoadingESMError);
runInWorker(cjsString, options); // does not reject

options = { eval: true, type: 'module' };
runInWorker(moduleString, options); // does not reject
assert.rejects(runInWorker(cjsString, options), esmLoadingCJSError);

options = { eval: true, type: 'wasm' };
assert.rejects(runInWorker(moduleString, options), invalidValueError);
assert.rejects(runInWorker(cjsString, options), invalidValueError);
19 changes: 19 additions & 0 deletions test/parallel/test-worker-type-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,22 @@ const { Worker } = require('worker_threads');
);
});
}

{
[
Symbol('test'),
{},
[],
() => {}
].forEach((type) => {
assert.throws(
() => new Worker('', { type, eval: true }),
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "options.type" property must be of type string.' +
common.invalidArgTypeHelper(type)
}
);
});
}

0 comments on commit 4ed31e6

Please sign in to comment.