summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYael Hermon <yaelhe@wix.com>2019-01-04 20:02:25 +0200
committerDaniel Bevenius <daniel.bevenius@gmail.com>2019-01-18 05:39:58 +0100
commit01cd21973b26a2cbacbe143c5983cb4adf8e7681 (patch)
tree16d2712ba9e73dfdb9612acbcde6e0bd8423e097
parent74562356db6964f8057ef4bd897725793e55d513 (diff)
downloadandroid-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.md6
-rw-r--r--doc/api/worker_threads.md9
-rw-r--r--lib/internal/errors.js3
-rw-r--r--lib/internal/worker.js13
-rw-r--r--src/env-inl.h5
-rw-r--r--src/env.h1
-rw-r--r--src/node_worker.cc67
-rw-r--r--src/node_worker.h5
-rw-r--r--test/parallel/test-internal-errors.js10
-rw-r--r--test/parallel/test-worker-execargv-invalid.js35
-rw-r--r--test/parallel/test-worker-execargv.js22
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,
diff --git a/src/env.h b/src/env.h
index fdd5c2382c..711d07963a 100644
--- a/src/env.h
+++ b/src/env.h
@@ -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');
+}