Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd6d0427

Browse files
joyeecheungmarco-ippolito
authored andcommitted
worker: allow copied NODE_OPTIONS in the env setting
When the worker spawning code copies NODE_OPTIONS from process.env,previously we do a check again on the copied NODE_OPTIONS and throwif it contains per-process settings. This can be problematicif the end user starts the process with a NODE_OPTIONS and theworker is spawned by a third-party that tries to extend process.envwith some overrides before passing them into the worker. This patchadds another exception that allows the per-process options in theNODE_OPTIONS passed to a worker if the NODE_OPTIONS ischaracter-by-character equal to the parent NODE_OPTIONS.While some more intelligent filter can be useful too,this works good enough for the inheritance case, when the workerspawning code does not intend to modify NODE_OPTIONS.PR-URL:#53596Refs:#41103Reviewed-By: James M Snell <jasnell@gmail.com>Reviewed-By: Xuguang Mei <meixuguang@gmail.com>Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent60ee41d commitd6d0427

File tree

4 files changed

+106
-6
lines changed

4 files changed

+106
-6
lines changed

‎src/node_worker.cc

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -549,12 +549,40 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
549549
// [0] is expected to be the program name, add dummy string.
550550
env_argv.insert(env_argv.begin(),"");
551551
std::vector<std::string> invalid_args{};
552-
options_parser::Parse(&env_argv,
553-
nullptr,
554-
&invalid_args,
555-
per_isolate_opts.get(),
556-
kAllowedInEnvvar,
557-
&errors);
552+
553+
std::string parent_node_options;
554+
USE(env->env_vars()->Get("NODE_OPTIONS").To(&parent_node_options));
555+
556+
// If the worker code passes { env: { ...process.env, ... } } or
557+
// the NODE_OPTIONS is otherwise character-for-character equal to the
558+
// original NODE_OPTIONS, allow per-process options inherited into
559+
// the worker since worker spawning code is not usually in charge of
560+
// how the NODE_OPTIONS is configured for the parent.
561+
// TODO(joyeecheung): a more intelligent filter may be more desirable.
562+
// but a string comparison is good enough(TM) for the case where the
563+
// worker spawning code just wants to pass the parent configuration down
564+
// and does not intend to modify NODE_OPTIONS.
565+
if (parent_node_options == node_options) {
566+
// Creates a wrapper per-process option over the per_isolate_opts
567+
// to allow per-process options copied from the parent.
568+
std::unique_ptr<PerProcessOptions> per_process_opts =
569+
std::make_unique<PerProcessOptions>();
570+
per_process_opts->per_isolate = per_isolate_opts;
571+
options_parser::Parse(&env_argv,
572+
nullptr,
573+
&invalid_args,
574+
per_process_opts.get(),
575+
kAllowedInEnvvar,
576+
&errors);
577+
}else {
578+
options_parser::Parse(&env_argv,
579+
nullptr,
580+
&invalid_args,
581+
per_isolate_opts.get(),
582+
kAllowedInEnvvar,
583+
&errors);
584+
}
585+
558586
if (!errors.empty() && args[1]->IsObject()) {
559587
// Only fail for explicitly provided env, this protects from failures
560588
// when NODE_OPTIONS from parent's env is used (which is the default).
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
// This test is meant to be spawned with NODE_OPTIONS=--title=foo
4+
constassert=require('assert');
5+
if(process.platform!=='sunos'){// --title is unsupported on SmartOS.
6+
assert.strictEqual(process.title,'foo');
7+
}
8+
9+
// Spawns a worker that may copy NODE_OPTIONS if it's set by the parent.
10+
const{ Worker}=require('worker_threads');
11+
newWorker(`require('assert').strictEqual(process.env.TEST_VAR, 'bar')`,{
12+
env:{
13+
...process.env,
14+
TEST_VAR:'bar',
15+
},
16+
eval:true,
17+
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
const{ Worker, isMainThread}=require('worker_threads')
4+
5+
// Tests that valid per-isolate/env NODE_OPTIONS are allowed and
6+
// work in child workers.
7+
if(isMainThread){
8+
newWorker(__filename,{
9+
env:{
10+
...process.env,
11+
NODE_OPTIONS:'--trace-exit'
12+
}
13+
})
14+
}else{
15+
setImmediate(()=>{
16+
process.nextTick(()=>{
17+
process.exit(0);
18+
});
19+
});
20+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use strict';
2+
3+
require('../common');
4+
const{
5+
spawnSyncAndExitWithoutError,
6+
spawnSyncAndAssert,
7+
}=require('../common/child_process');
8+
constfixtures=require('../common/fixtures');
9+
spawnSyncAndExitWithoutError(
10+
process.execPath,
11+
[
12+
fixtures.path('spawn-worker-with-copied-env'),
13+
],
14+
{
15+
env:{
16+
...process.env,
17+
NODE_OPTIONS:'--title=foo'
18+
}
19+
}
20+
);
21+
22+
spawnSyncAndAssert(
23+
process.execPath,
24+
[
25+
fixtures.path('spawn-worker-with-trace-exit'),
26+
],
27+
{
28+
env:{
29+
...process.env,
30+
}
31+
},
32+
{
33+
stderr:/spawn-worker-with-trace-exit\.js:17/
34+
}
35+
);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp