diff options
author | Yael Hermon <yaelhe@wix.com> | 2019-01-04 20:02:25 +0200 |
---|---|---|
committer | Daniel Bevenius <daniel.bevenius@gmail.com> | 2019-01-18 05:39:58 +0100 |
commit | 01cd21973b26a2cbacbe143c5983cb4adf8e7681 (patch) | |
tree | 16d2712ba9e73dfdb9612acbcde6e0bd8423e097 | |
parent | 74562356db6964f8057ef4bd897725793e55d513 (diff) | |
download | android-node-v8-01cd21973b26a2cbacbe143c5983cb4adf8e7681.tar.gz android-node-v8-01cd21973b26a2cbacbe143c5983cb4adf8e7681.tar.bz2 android-node-v8-01cd21973b26a2cbacbe143c5983cb4adf8e7681.zip |
worker: enable passing command line flags
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: https://github.com/nodejs/node/pull/25467
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
-rw-r--r-- | doc/api/errors.md | 6 | ||||
-rw-r--r-- | doc/api/worker_threads.md | 9 | ||||
-rw-r--r-- | lib/internal/errors.js | 3 | ||||
-rw-r--r-- | lib/internal/worker.js | 13 | ||||
-rw-r--r-- | src/env-inl.h | 5 | ||||
-rw-r--r-- | src/env.h | 1 | ||||
-rw-r--r-- | src/node_worker.cc | 67 | ||||
-rw-r--r-- | src/node_worker.h | 5 | ||||
-rw-r--r-- | test/parallel/test-internal-errors.js | 10 | ||||
-rw-r--r-- | test/parallel/test-worker-execargv-invalid.js | 35 | ||||
-rw-r--r-- | test/parallel/test-worker-execargv.js | 22 |
11 files changed, 167 insertions, 9 deletions
diff --git a/doc/api/errors.md b/doc/api/errors.md index 840ba9b85c..96ba39dd48 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1909,6 +1909,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 diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md index 78f35412c3..05cafa9cb1 100644 --- a/doc/api/worker_threads.md +++ b/doc/api/worker_threads.md @@ -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 diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 12b01ffb9b..b33cd27109 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -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"', diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 2e1568a738..e20a37af8a 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -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'); @@ -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('./') && @@ -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)); diff --git a/src/env-inl.h b/src/env-inl.h index 7c643539ad..d5bf6bc086 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -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, @@ -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) diff --git a/src/node_worker.cc b/src/node_worker.cc index dab7945aae..30004957a4 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -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; @@ -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. { @@ -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_); @@ -391,14 +399,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) { diff --git a/src/node_worker.h b/src/node_worker.h index cbd4a86157..dd054ac38d 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -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. diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js index bf4688d89b..e53a9a9fe4 100644 --- a/test/parallel/test-internal-errors.js +++ b/test/parallel/test-internal-errors.js @@ -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' + ); +} diff --git a/test/parallel/test-worker-execargv-invalid.js b/test/parallel/test-worker-execargv-invalid.js new file mode 100644 index 0000000000..e2deaf5464 --- /dev/null +++ b/test/parallel/test-worker-execargv-invalid.js @@ -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); +} diff --git a/test/parallel/test-worker-execargv.js b/test/parallel/test-worker-execargv.js new file mode 100644 index 0000000000..e04f015e03 --- /dev/null +++ b/test/parallel/test-worker-execargv.js @@ -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'); +} |