Skip to content

Commit

Permalink
worker: enable passing command line flags
Browse files Browse the repository at this point in the history
This PR adds the ability to provide Workers with their own
execArgv flags in replacement of the main thread's execArgv. Only
per-Isolate/per-Environment options are allowed. Per-Process options
and V8 flags are not allowed. Passing an empty execArgv array will
reset per-Isolate and per-Environment options of the Worker to their
defaults. If execArgv option is not passed, the Worker will get
the same flags as the main thread.

Usage example:
```
const worker = new Worker(__filename, {
    execArgv: ['--trace-warnings'],
});
```

PR-URL: #25467
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
  • Loading branch information
yaelhe authored and targos committed Jan 24, 2019
1 parent da8c526 commit 219b1b8
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 9 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,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.
V8 options (such as `--max-old-space-size`) and options that affect the
process (such as `--title`) are not supported.

### 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 @@ -992,6 +992,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 @@ -392,6 +392,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 @@ -9,7 +9,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 @@ -67,7 +69,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 @@ -112,6 +117,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 @@ -390,14 +398,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');
}

0 comments on commit 219b1b8

Please sign in to comment.