Skip to content

Commit d53e536

Browse files
theanarkhtargos
authored andcommitted
src: fix execArgv in worker
PR-URL: #53029 Fixes: #53011 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 21776a3 commit d53e536

File tree

2 files changed

+22
-6
lines changed

2 files changed

+22
-6
lines changed

src/node_worker.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
625625
exec_argv.push_back(arg_string);
626626
}
627627
} else {
628-
exec_argv_out = env->exec_argv();
629628
exec_argv.insert(
630-
exec_argv.end(), exec_argv_out.begin(), exec_argv_out.end());
629+
exec_argv.end(), env->exec_argv().begin(), env->exec_argv().end());
631630
}
632631

633632
std::vector<std::string> invalid_args{};
@@ -643,7 +642,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
643642

644643
// The first argument is program name.
645644
invalid_args.erase(invalid_args.begin());
646-
if (errors.size() > 0 || invalid_args.size() > 0) {
645+
// Only fail for explicitly provided execArgv, this protects from failures
646+
// when execArgv from parent's execArgv is used (which is the default).
647+
if (errors.size() > 0 || (invalid_args.size() > 0 && args[2]->IsArray())) {
647648
Local<Value> error;
648649
if (!ToV8Value(env->context(), errors.size() > 0 ? errors : invalid_args)
649650
.ToLocal(&error)) {
Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,31 @@
1-
// Flags: --expose-internals
1+
// Flags: --expose-internals --expose-gc
22
'use strict';
33
require('../common');
44
const { Worker } = require('worker_threads');
5+
const assert = require('assert');
56

67
const CODE = `
78
// If the --expose-internals flag does not pass to worker
89
// require function will throw an error
910
require('internal/options');
11+
global.gc();
1012
`;
11-
// Test if the flags is passed to worker threads
13+
14+
// Test if the flags is passed to worker threads correctly
15+
// and do not throw an error with the invalid execArgv
16+
// when execArgv is inherited from parent
1217
// See https://github.com/nodejs/node/issues/52825
18+
// See https://github.com/nodejs/node/issues/53011
19+
20+
// Inherited env, execArgv from the parent will be ok
1321
new Worker(CODE, { eval: true });
14-
new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] });
22+
// Pass process.env explicitly and inherited execArgv from parent will be ok
1523
new Worker(CODE, { eval: true, env: process.env });
24+
// Inherited env from the parent and pass execArgv (Node.js options) explicitly will be ok
1625
new Worker(CODE, { eval: true, execArgv: ['--expose-internals'] });
26+
// Pass process.env and execArgv (Node.js options) explicitly will be ok
27+
new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] });
28+
// Pass execArgv (V8 options) explicitly will throw an error
29+
assert.throws(() => {
30+
new Worker(CODE, { eval: true, execArgv: ['--expose-gc'] });
31+
}, /ERR_WORKER_INVALID_EXEC_ARGV/);

0 commit comments

Comments
 (0)