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: enable passing command line flags #25467

Closed
wants to merge 3 commits 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
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,12 @@ The fulfilled value of a linking promise is not a `vm.SourceTextModule` object.
The current module's status does not allow for this operation. The specific
meaning of the error depends on the specific function.

<a id="ERR_WORKER_INVALID_EXEC_ARGV"></a>
### ERR_WORKER_INVALID_EXEC_ARGV

The `execArgv` option passed to the `Worker` constructor contains
invalid flags.

<a id="ERR_WORKER_PATH"></a>
### ERR_WORKER_PATH

Expand Down
9 changes: 6 additions & 3 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,16 @@ if (isMainThread) {
occur as described in the [HTML structured clone algorithm][], and an error
will be thrown if the object cannot be cloned (e.g. because it contains
`function`s).
* stdin {boolean} If this is set to `true`, then `worker.stdin` will
* `stdin` {boolean} If this is set to `true`, then `worker.stdin` will
provide a writable stream whose contents will appear as `process.stdin`
inside the Worker. By default, no data is provided.
* stdout {boolean} If this is set to `true`, then `worker.stdout` will
* `stdout` {boolean} If this is set to `true`, then `worker.stdout` will
not automatically be piped through to `process.stdout` in the parent.
* stderr {boolean} If this is set to `true`, then `worker.stderr` will
* `stderr` {boolean} If this is set to `true`, then `worker.stderr` will
not automatically be piped through to `process.stderr` in the parent.
* `execArgv` {string[]} List of node CLI options passed to the worker.
yaelhe marked this conversation as resolved.
Show resolved Hide resolved
V8 options (such as `--max-old-space-size`) and options that affect the
process (such as `--title`) are not supported.

jdalton marked this conversation as resolved.
Show resolved Hide resolved
### Event: 'error'
<!-- YAML
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,9 @@ E('ERR_VM_MODULE_NOT_LINKED',
E('ERR_VM_MODULE_NOT_MODULE',
'Provided module is not an instance of Module', Error);
E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
E('ERR_WORKER_INVALID_EXEC_ARGV', (errors) =>
`Initiated Worker with invalid execArgv flags: ${errors.join(', ')}`,
Error);
E('ERR_WORKER_PATH',
'The worker script filename must be an absolute path or a relative ' +
'path starting with \'./\' or \'../\'. Received "%s"',
Expand Down
13 changes: 11 additions & 2 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const {
ERR_WORKER_PATH,
ERR_WORKER_UNSERIALIZABLE_ERROR,
ERR_WORKER_UNSUPPORTED_EXTENSION,
ERR_WORKER_INVALID_EXEC_ARGV,
ERR_INVALID_ARG_TYPE,
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');

Expand Down Expand Up @@ -49,7 +51,11 @@ class Worker extends EventEmitter {
super();
debug(`[${threadId}] create new worker`, filename, options);
validateString(filename, 'filename');

if (options.execArgv && !Array.isArray(options.execArgv)) {
throw new ERR_INVALID_ARG_TYPE('options.execArgv',
'array',
options.execArgv);
}
if (!options.eval) {
if (!path.isAbsolute(filename) &&
!filename.startsWith('./') &&
Expand All @@ -68,7 +74,10 @@ class Worker extends EventEmitter {

const url = options.eval ? null : pathToFileURL(filename);
// Set up the C++ handle for the worker, as well as some internal wiring.
this[kHandle] = new WorkerImpl(url);
this[kHandle] = new WorkerImpl(url, options.execArgv);
if (this[kHandle].invalidExecArgv) {
throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
}
this[kHandle].onexit = (code) => this[kOnExit](code);
this[kPort] = this[kHandle].messagePort;
this[kPort].on('message', (data) => this[kOnMessage](data));
Expand Down
5 changes: 5 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,11 @@ inline std::shared_ptr<PerIsolateOptions> IsolateData::options() {
return options_;
}

inline void IsolateData::set_options(
std::shared_ptr<PerIsolateOptions> options) {
options_ = options;
}

void Environment::CreateImmediate(native_immediate_callback cb,
void* data,
v8::Local<v8::Object> obj,
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ class IsolateData {
inline uint32_t* zero_fill_field() const;
inline MultiIsolatePlatform* platform() const;
inline std::shared_ptr<PerIsolateOptions> options();
inline void set_options(std::shared_ptr<PerIsolateOptions> options);

#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
Expand Down
67 changes: 64 additions & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include "async_wrap-inl.h"

#include <string>
#include <vector>

using node::options_parser::kDisallowedInEnvironment;
using v8::ArrayBuffer;
using v8::Context;
using v8::Function;
Expand Down Expand Up @@ -68,7 +70,10 @@ void WaitForWorkerInspectorToStop(Environment* child) {}

} // anonymous namespace

Worker::Worker(Environment* env, Local<Object> wrap, const std::string& url)
Worker::Worker(Environment* env,
Local<Object> wrap,
const std::string& url,
std::shared_ptr<PerIsolateOptions> per_isolate_opts)
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER), url_(url) {
// Generate a new thread id.
{
Expand Down Expand Up @@ -113,6 +118,9 @@ Worker::Worker(Environment* env, Local<Object> wrap, const std::string& url)
&loop_,
env->isolate_data()->platform(),
array_buffer_allocator_.get()));
if (per_isolate_opts != nullptr) {
isolate_data_->set_options(per_isolate_opts);
}
CHECK(isolate_data_);

Local<Context> context = NewContext(isolate_);
Expand Down Expand Up @@ -389,14 +397,67 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
}

std::string url;
std::shared_ptr<PerIsolateOptions> per_isolate_opts = nullptr;

// Argument might be a string or URL
if (args.Length() == 1 && !args[0]->IsNullOrUndefined()) {
if (args.Length() > 0 && !args[0]->IsNullOrUndefined()) {
Utf8Value value(
args.GetIsolate(),
args[0]->ToString(env->context()).FromMaybe(v8::Local<v8::String>()));
url.append(value.out(), value.length());

if (args.Length() > 1 && args[1]->IsArray()) {
v8::Local<v8::Array> array = args[1].As<v8::Array>();
// The first argument is reserved for program name, but we don't need it
// in workers.
std::vector<std::string> exec_argv = {""};
uint32_t length = array->Length();
for (uint32_t i = 0; i < length; i++) {
v8::Local<v8::Value> arg;
if (!array->Get(env->context(), i).ToLocal(&arg)) {
return;
}
v8::MaybeLocal<v8::String> arg_v8_string =
arg->ToString(env->context());
if (arg_v8_string.IsEmpty()) {
return;
}
Utf8Value arg_utf8_value(
args.GetIsolate(),
arg_v8_string.FromMaybe(v8::Local<v8::String>()));
std::string arg_string(arg_utf8_value.out(), arg_utf8_value.length());
exec_argv.push_back(arg_string);
}

std::vector<std::string> invalid_args{};
std::vector<std::string> errors{};
per_isolate_opts.reset(new PerIsolateOptions());

// Using invalid_args as the v8_args argument as it stores unknown
// options for the per isolate parser.
options_parser::PerIsolateOptionsParser::instance.Parse(
&exec_argv,
nullptr,
&invalid_args,
per_isolate_opts.get(),
kDisallowedInEnvironment,
&errors);

// The first argument is program name.
invalid_args.erase(invalid_args.begin());
if (errors.size() > 0 || invalid_args.size() > 0) {
v8::Local<v8::Value> value =
ToV8Value(env->context(),
errors.size() > 0 ? errors : invalid_args)
.ToLocalChecked();
Local<String> key =
FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv");
args.This()->Set(env->context(), key, value).FromJust();
return;
}
}
}
new Worker(env, args.This(), url);
new Worker(env, args.This(), url, per_isolate_opts);
}

void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
Expand Down
5 changes: 4 additions & 1 deletion src/node_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ namespace worker {
// A worker thread, as represented in its parent thread.
class Worker : public AsyncWrap {
public:
Worker(Environment* env, v8::Local<v8::Object> wrap, const std::string& url);
Worker(Environment* env,
v8::Local<v8::Object> wrap,
const std::string& url,
std::shared_ptr<PerIsolateOptions> per_isolate_opts);
~Worker();

// Run the worker. This is only called from the worker thread.
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,13 @@ assert.strictEqual(

restoreStdout();
}

{
const error = new errors.codes.ERR_WORKER_INVALID_EXEC_ARGV(
['--foo, --bar']
);
assert.strictEqual(
error.message,
'Initiated Worker with invalid execArgv flags: --foo, --bar'
);
}
35 changes: 35 additions & 0 deletions test/parallel/test-worker-execargv-invalid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const { Worker } = require('worker_threads');

{
const expectedErr = common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError
}, 2);

assert.throws(() => {
new Worker(__filename, { execArgv: 'hello' });
}, expectedErr);
assert.throws(() => {
new Worker(__filename, { execArgv: 6 });
}, expectedErr);
}

{
const expectedErr = common.expectsError({
code: 'ERR_WORKER_INVALID_EXEC_ARGV',
type: Error
}, 3);
assert.throws(() => {
new Worker(__filename, { execArgv: ['--foo'] });
}, expectedErr);
assert.throws(() => {
new Worker(__filename, { execArgv: ['--title=blah'] });
}, expectedErr);
assert.throws(() => {
new Worker(__filename, { execArgv: ['--redirect-warnings'] });
}, expectedErr);
}
22 changes: 22 additions & 0 deletions test/parallel/test-worker-execargv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';
const common = require('../common');
const assert = require('assert');

// This test ensures that Workers have the ability to get
// their own command line flags.

const { Worker, isMainThread } = require('worker_threads');
const { StringDecoder } = require('string_decoder');
const decoder = new StringDecoder('utf8');

if (isMainThread) {
const w = new Worker(__filename, { execArgv: ['--trace-warnings'] });
w.stderr.on('data', common.mustCall((chunk) => {
const error = decoder.write(chunk);
assert.ok(
/Warning: some warning[\s\S]*at Object\.<anonymous>/.test(error)
);
}));
} else {
process.emitWarning('some warning');
}