Skip to content

Commit 3f44cb6

Browse files
legendecaslegendecas
committed
src: reject per-process isolate flags in workers
V8 flags are saved in a per-process storages and were ignored in the Worker constructor options. Reject V8 flags in the Worker constructor proactively to indicate these options should be set at process level.
1 parent c0962dc commit 3f44cb6

File tree

4 files changed

+67
-62
lines changed

4 files changed

+67
-62
lines changed

‎src/api/environment.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,7 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
262262
isolate->SetWasmStreamingCallback(wasm_web_api::StartStreamingCompilation);
263263
}
264264

265-
if (per_process::cli_options->get_per_isolate_options()
266-
->experimental_shadow_realm) {
265+
if (per_process::cli_options->experimental_shadow_realm) {
267266
isolate->SetHostCreateShadowRealmContextCallback(
268267
shadow_realm::HostCreateShadowRealmContextCallback);
269268
}

‎src/node_options.cc‎

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
485485
AddOption("--experimental-report", "", NoOp{}, kAllowedInEnvvar);
486486
AddOption(
487487
"--experimental-wasi-unstable-preview1", "", NoOp{}, kAllowedInEnvvar);
488-
AddOption("--expose-gc", "expose gc extension", V8Option{}, kAllowedInEnvvar);
489488
AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
490489
AddOption("--frozen-intrinsics",
491490
"experimental frozen intrinsics support",
@@ -565,9 +564,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
565564
"preserve symbolic links when resolving the main module",
566565
&EnvironmentOptions::preserve_symlinks_main,
567566
kAllowedInEnvvar);
568-
AddOption("--prof",
569-
"Generate V8 profiler output.",
570-
V8Option{});
571567
AddOption("--prof-process",
572568
"process V8 profiler output generated using --prof",
573569
&EnvironmentOptions::prof_process);
@@ -820,42 +816,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
820816

821817
PerIsolateOptionsParser::PerIsolateOptionsParser(
822818
const EnvironmentOptionsParser& eop) {
819+
// Generally, V8 flags are saved in per-process storage and should be added
820+
// to PerProcessOptionsParser.
821+
823822
AddOption("--track-heap-objects",
824823
"track heap object allocations for heap snapshots",
825824
&PerIsolateOptions::track_heap_objects,
826825
kAllowedInEnvvar);
827826

828-
// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
829-
AddOption("--abort-on-uncaught-exception",
830-
"aborting instead of exiting causes a core file to be generated "
831-
"for analysis",
832-
V8Option{},
833-
kAllowedInEnvvar);
834-
AddOption("--interpreted-frames-native-stack",
835-
"help system profilers to translate JavaScript interpreted frames",
836-
V8Option{},
837-
kAllowedInEnvvar);
838-
AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvvar);
839-
AddOption("--max-semi-space-size", "", V8Option{}, kAllowedInEnvvar);
840-
AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvvar);
841-
AddOption(
842-
"--perf-basic-prof-only-functions", "", V8Option{}, kAllowedInEnvvar);
843-
AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvvar);
844-
AddOption("--perf-prof-unwinding-info", "", V8Option{}, kAllowedInEnvvar);
845-
AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvvar);
846-
AddOption("--disallow-code-generation-from-strings",
847-
"disallow eval and friends",
848-
V8Option{},
849-
kAllowedInEnvvar);
850-
AddOption("--huge-max-old-generation-size",
851-
"increase default maximum heap size on machines with 16GB memory "
852-
"or more",
853-
V8Option{},
854-
kAllowedInEnvvar);
855-
AddOption("--jitless",
856-
"disable runtime allocation of executable memory",
857-
V8Option{},
858-
kAllowedInEnvvar);
859827
AddOption("--report-uncaught-exception",
860828
"generate diagnostic report on uncaught exceptions",
861829
&PerIsolateOptions::report_uncaught_exception,
@@ -870,21 +838,7 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
870838
&PerIsolateOptions::report_signal,
871839
kAllowedInEnvvar);
872840
Implies("--report-signal", "--report-on-signal");
873-
AddOption("--enable-etw-stack-walking",
874-
"provides heap data to ETW Windows native tracing",
875-
V8Option{},
876-
kAllowedInEnvvar);
877-
878-
AddOption("--experimental-top-level-await", "", NoOp{}, kAllowedInEnvvar);
879841

880-
AddOption("--experimental-shadow-realm",
881-
"",
882-
&PerIsolateOptions::experimental_shadow_realm,
883-
kAllowedInEnvvar);
884-
AddOption("--harmony-shadow-realm", "", V8Option{});
885-
Implies("--experimental-shadow-realm", "--harmony-shadow-realm");
886-
Implies("--harmony-shadow-realm", "--experimental-shadow-realm");
887-
ImpliesNot("--no-harmony-shadow-realm", "--experimental-shadow-realm");
888842
AddOption("--build-snapshot",
889843
"Generate a snapshot blob when the process exits.",
890844
&PerIsolateOptions::build_snapshot,
@@ -1092,6 +1046,58 @@ PerProcessOptionsParser::PerProcessOptionsParser(
10921046
"performance.",
10931047
&PerProcessOptions::disable_wasm_trap_handler,
10941048
kAllowedInEnvvar);
1049+
1050+
// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
1051+
// V8 flags are be per-process options and should not be modified with
1052+
// NODE_OPTIONS or execArgv with a Worker constructor.
1053+
AddOption("--abort-on-uncaught-exception",
1054+
"aborting instead of exiting causes a core file to be generated "
1055+
"for analysis",
1056+
V8Option{},
1057+
kAllowedInEnvvar);
1058+
AddOption("--expose-gc", "expose gc extension", V8Option{}, kAllowedInEnvvar);
1059+
AddOption("--interpreted-frames-native-stack",
1060+
"help system profilers to translate JavaScript interpreted frames",
1061+
V8Option{},
1062+
kAllowedInEnvvar);
1063+
AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvvar);
1064+
AddOption("--max-semi-space-size", "", V8Option{}, kAllowedInEnvvar);
1065+
AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvvar);
1066+
AddOption(
1067+
"--perf-basic-prof-only-functions", "", V8Option{}, kAllowedInEnvvar);
1068+
AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvvar);
1069+
AddOption("--perf-prof-unwinding-info", "", V8Option{}, kAllowedInEnvvar);
1070+
AddOption("--prof", "Generate V8 profiler output.", V8Option{});
1071+
AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvvar);
1072+
AddOption("--disallow-code-generation-from-strings",
1073+
"disallow eval and friends",
1074+
V8Option{},
1075+
kAllowedInEnvvar);
1076+
AddOption("--huge-max-old-generation-size",
1077+
"increase default maximum heap size on machines with 16GB memory "
1078+
"or more",
1079+
V8Option{},
1080+
kAllowedInEnvvar);
1081+
AddOption("--jitless",
1082+
"disable runtime allocation of executable memory",
1083+
V8Option{},
1084+
kAllowedInEnvvar);
1085+
AddOption("--enable-etw-stack-walking",
1086+
"provides heap data to ETW Windows native tracing",
1087+
V8Option{},
1088+
kAllowedInEnvvar);
1089+
1090+
AddOption("--experimental-top-level-await", "", NoOp{}, kAllowedInEnvvar);
1091+
1092+
AddOption("--experimental-shadow-realm",
1093+
"",
1094+
&PerProcessOptions::experimental_shadow_realm,
1095+
kAllowedInEnvvar);
1096+
AddOption("--harmony-shadow-realm", "", V8Option{});
1097+
1098+
Implies("--experimental-shadow-realm", "--harmony-shadow-realm");
1099+
Implies("--harmony-shadow-realm", "--experimental-shadow-realm");
1100+
ImpliesNot("--no-harmony-shadow-realm", "--experimental-shadow-realm");
10951101
}
10961102

10971103
inline std::string RemoveBrackets(const std::string& host) {

‎src/node_options.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,6 @@ class PerIsolateOptions : public Options {
249249
bool track_heap_objects = false;
250250
bool report_uncaught_exception = false;
251251
bool report_on_signal = false;
252-
bool experimental_shadow_realm = false;
253252
std::string report_signal = "SIGUSR2";
254253
bool build_snapshot = false;
255254
std::string build_snapshot_config;
@@ -289,6 +288,7 @@ class PerProcessOptions : public Options {
289288
bool print_v8_help = false;
290289
bool print_version = false;
291290
std::string experimental_sea_config;
291+
bool experimental_shadow_realm = false;
292292
std::string run;
293293

294294
#ifdef NODE_HAVE_I18N_SUPPORT

‎test/parallel/test-cli-node-options.js‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,20 @@ if (common.hasCrypto) {
6767
}
6868

6969
// V8 options
70-
expect('--abort_on-uncaught_exception', 'B\n');
71-
expect('--disallow-code-generation-from-strings', 'B\n');
72-
expect('--expose-gc', 'B\n');
73-
expect('--huge-max-old-generation-size', 'B\n');
74-
expect('--jitless', 'B\n');
75-
expect('--max-old-space-size=0', 'B\n');
76-
expect('--max-semi-space-size=0', 'B\n');
77-
expect('--stack-trace-limit=100',
78-
/(\s*at f \(\[(eval|worker eval)\]:1:\d*\)\r?\n)/,
70+
expectNoWorker('--abort_on-uncaught_exception', 'B\n');
71+
expectNoWorker('--disallow-code-generation-from-strings', 'B\n');
72+
expectNoWorker('--expose-gc', 'B\n');
73+
expectNoWorker('--huge-max-old-generation-size', 'B\n');
74+
expectNoWorker('--jitless', 'B\n');
75+
expectNoWorker('--max-old-space-size=0', 'B\n');
76+
expectNoWorker('--max-semi-space-size=0', 'B\n');
77+
expectNoWorker('--stack-trace-limit=100',
78+
/(\s*at f \(\[eval\]:1:\d*\)\r?\n)/,
7979
'(function f() { f(); })();',
8080
true);
8181
// Unsupported on arm. See https://crbug.com/v8/8713.
8282
if (!['arm', 'arm64'].includes(process.arch))
83-
expect('--interpreted-frames-native-stack', 'B\n');
83+
expectNoWorker('--interpreted-frames-native-stack', 'B\n');
8484

8585
// Workers can't eval as ES Modules. https://github.com/nodejs/node/issues/30682
8686
expectNoWorker('--input-type=module',

0 commit comments

Comments
 (0)