Skip to content

Commit

Permalink
src: fix execArgv in worker
Browse files Browse the repository at this point in the history
  • Loading branch information
theanarkh committed May 16, 2024
1 parent 2693f09 commit a5961c7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& 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<std::string> invalid_args{};
Expand All @@ -608,7 +607,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& 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<Value> error;
if (!ToV8Value(env->context(), errors.size() > 0 ? errors : invalid_args)
.ToLocal(&error)) {
Expand Down
19 changes: 16 additions & 3 deletions test/parallel/test-worker-cli-options.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
// Flags: --expose-internals
// Flags: --expose-internals --expose-gc
'use strict';
require('../common');
const { Worker } = require('worker_threads');
const common = require('../common');

Check failure on line 5 in test/parallel/test-worker-cli-options.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'common' is assigned a value but never used
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');
`;
// Test if the flags is passed to worker threads
// Test if the flags is passed to worker threads collectly
// 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(Node.js options) 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'] })

Check failure on line 28 in test/parallel/test-worker-cli-options.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 4

Check failure on line 28 in test/parallel/test-worker-cli-options.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing semicolon
}, /ERR_WORKER_INVALID_EXEC_ARGV/);

0 comments on commit a5961c7

Please sign in to comment.