diff --git a/src/node_worker.cc b/src/node_worker.cc index f744c6a1a97ff1..f12dde0ae50a6c 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -549,12 +549,40 @@ void Worker::New(const FunctionCallbackInfo& args) { // [0] is expected to be the program name, add dummy string. env_argv.insert(env_argv.begin(), ""); std::vector invalid_args{}; - options_parser::Parse(&env_argv, - nullptr, - &invalid_args, - per_isolate_opts.get(), - kAllowedInEnvvar, - &errors); + + std::string parent_node_options; + USE(env->env_vars()->Get("NODE_OPTIONS").To(&parent_node_options)); + + // If the worker code passes { env: { ...process.env, ... } } or + // the NODE_OPTIONS is otherwise character-for-character equal to the + // original NODE_OPTIONS, allow per-process options inherited into + // the worker since worker spawning code is not usually in charge of + // how the NODE_OPTIONS is configured for the parent. + // TODO(joyeecheung): a more intelligent filter may be more desirable. + // but a string comparison is good enough(TM) for the case where the + // worker spawning code just wants to pass the parent configuration down + // and does not intend to modify NODE_OPTIONS. + if (parent_node_options == node_options) { + // Creates a wrapper per-process option over the per_isolate_opts + // to allow per-process options copied from the parent. + std::unique_ptr per_process_opts = + std::make_unique(); + per_process_opts->per_isolate = per_isolate_opts; + options_parser::Parse(&env_argv, + nullptr, + &invalid_args, + per_process_opts.get(), + kAllowedInEnvvar, + &errors); + } else { + options_parser::Parse(&env_argv, + nullptr, + &invalid_args, + per_isolate_opts.get(), + kAllowedInEnvvar, + &errors); + } + if (!errors.empty() && args[1]->IsObject()) { // Only fail for explicitly provided env, this protects from failures // when NODE_OPTIONS from parent's env is used (which is the default). diff --git a/test/fixtures/spawn-worker-with-copied-env.js b/test/fixtures/spawn-worker-with-copied-env.js new file mode 100644 index 00000000000000..51e64bd8755b6d --- /dev/null +++ b/test/fixtures/spawn-worker-with-copied-env.js @@ -0,0 +1,17 @@ +'use strict'; + +// This test is meant to be spawned with NODE_OPTIONS=--title=foo +const assert = require('assert'); +if (process.platform !== 'sunos') { // --title is unsupported on SmartOS. + assert.strictEqual(process.title, 'foo'); +} + +// Spawns a worker that may copy NODE_OPTIONS if it's set by the parent. +const { Worker } = require('worker_threads'); +new Worker(`require('assert').strictEqual(process.env.TEST_VAR, 'bar')`, { + env: { + ...process.env, + TEST_VAR: 'bar', + }, + eval: true, +}); diff --git a/test/fixtures/spawn-worker-with-trace-exit.js b/test/fixtures/spawn-worker-with-trace-exit.js new file mode 100644 index 00000000000000..6cf091c727a74f --- /dev/null +++ b/test/fixtures/spawn-worker-with-trace-exit.js @@ -0,0 +1,20 @@ +'use strict'; + +const { Worker, isMainThread } = require('worker_threads') + +// Tests that valid per-isolate/env NODE_OPTIONS are allowed and +// work in child workers. +if (isMainThread) { + new Worker(__filename, { + env: { + ...process.env, + NODE_OPTIONS: '--trace-exit' + } + }) +} else { + setImmediate(() => { + process.nextTick(() => { + process.exit(0); + }); + }); +} diff --git a/test/parallel/test-worker-node-options.js b/test/parallel/test-worker-node-options.js new file mode 100644 index 00000000000000..7a26154e2f4800 --- /dev/null +++ b/test/parallel/test-worker-node-options.js @@ -0,0 +1,35 @@ +'use strict'; + +require('../common'); +const { + spawnSyncAndExitWithoutError, + spawnSyncAndAssert, +} = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +spawnSyncAndExitWithoutError( + process.execPath, + [ + fixtures.path('spawn-worker-with-copied-env'), + ], + { + env: { + ...process.env, + NODE_OPTIONS: '--title=foo' + } + } +); + +spawnSyncAndAssert( + process.execPath, + [ + fixtures.path('spawn-worker-with-trace-exit'), + ], + { + env: { + ...process.env, + } + }, + { + stderr: /spawn-worker-with-trace-exit\.js:17/ + } +);