From 79986bcd203eae42d81f0ff2b550cd8347a2c7b4 Mon Sep 17 00:00:00 2001 From: theanarkh Date: Fri, 17 May 2024 02:31:10 +0800 Subject: [PATCH] src: fix execArgv in worker --- src/node_worker.cc | 7 ++++--- test/parallel/test-worker-cli-options.js | 21 ++++++++++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index 14fcf72c1ec3e5..bc43cb42934ba9 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -590,9 +590,8 @@ void Worker::New(const FunctionCallbackInfo& args) { exec_argv.push_back(arg_string); } } else { - exec_argv_out = env->exec_argv(); exec_argv.insert( - exec_argv.end(), exec_argv_out.begin(), exec_argv_out.end()); + exec_argv.end(), env->exec_argv().begin(), env->exec_argv().end()); } std::vector invalid_args{}; @@ -608,7 +607,9 @@ void Worker::New(const FunctionCallbackInfo& args) { // The first argument is program name. invalid_args.erase(invalid_args.begin()); - if (errors.size() > 0 || invalid_args.size() > 0) { + // Only fail for explicitly provided execArgv, this protects from failures + // when execArgv from parent's execArgv is used (which is the default). + if (errors.size() > 0 || (invalid_args.size() > 0 && args[2]->IsArray())) { Local error; if (!ToV8Value(env->context(), errors.size() > 0 ? errors : invalid_args) .ToLocal(&error)) { diff --git a/test/parallel/test-worker-cli-options.js b/test/parallel/test-worker-cli-options.js index 98682243f2d16c..ae59dcb6e62d48 100644 --- a/test/parallel/test-worker-cli-options.js +++ b/test/parallel/test-worker-cli-options.js @@ -1,16 +1,31 @@ -// Flags: --expose-internals +// Flags: --expose-internals --expose-gc 'use strict'; require('../common'); const { Worker } = require('worker_threads'); +const assert = require('assert'); const CODE = ` // If the --expose-internals flag does not pass to worker // require function will throw an error require('internal/options'); +global.gc(); `; -// Test if the flags is passed to worker threads + +// Test if the flags is passed to worker threads correctly +// and do not throw an error with the invalid execArgv +// when execArgv is inherited from parent // See https://github.com/nodejs/node/issues/52825 +// See https://github.com/nodejs/node/issues/53011 + +// Inherited env, execArgv from the parent will be ok new Worker(CODE, { eval: true }); -new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] }); +// Pass process.env explicitly and inherited execArgv from parent will be ok new Worker(CODE, { eval: true, env: process.env }); +// Inherited env from the parent and pass execArgv (Node.js options) explicitly will be ok new Worker(CODE, { eval: true, execArgv: ['--expose-internals'] }); +// Pass process.env and execArgv (Node.js options) explicitly will be ok +new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] }); +// Pass execArgv (V8 options) explicitly will throw an error +assert.throws(() => { + new Worker(CODE, { eval: true, execArgv: ['--expose-gc'] }); +}, /ERR_WORKER_INVALID_EXEC_ARGV/);