From c15afda79f7e0f4318848649ef43c78fea46d0ba Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 7 Oct 2021 12:03:10 +0800 Subject: [PATCH] src: get embedder options on-demand Only query embedder options when they are needed so that the bootstrap remains as stateless as possible so that the bootstrap snapshot is controlled solely by configure options, and subsequent runtime changes should be done in pre-execution. PR-URL: https://github.com/nodejs/node/pull/40357 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Shelley Vohr --- lib/internal/bootstrap/pre_execution.js | 7 ++--- lib/internal/options.js | 28 ++++++++++------- src/node_options.cc | 41 ++++++++++++++++--------- src/node_options.h | 2 +- 4 files changed, 48 insertions(+), 30 deletions(-) diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index a4c73cba5481b3..f2a10641906e31 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -11,8 +11,7 @@ const { const { getOptionValue, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getEmbedderOptions, } = require('internal/options'); const { reconnectZeroFillToggle } = require('internal/buffer'); @@ -421,7 +420,7 @@ function initializeWASI() { function initializeCJSLoader() { const CJSLoader = require('internal/modules/cjs/loader'); - if (!noGlobalSearchPaths) { + if (!getEmbedderOptions().noGlobalSearchPaths) { CJSLoader.Module._initPaths(); } // TODO(joyeecheung): deprecate this in favor of a proper hook? @@ -433,7 +432,7 @@ function initializeESMLoader() { // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. internalBinding('module_wrap').callbackMap = new SafeWeakMap(); - if (shouldNotRegisterESMLoader) return; + if (getEmbedderOptions().shouldNotRegisterESMLoader) return; const { setImportModuleDynamicallyCallback, diff --git a/lib/internal/options.js b/lib/internal/options.js index cb694c7dfdd576..01b334d4ec5614 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -1,36 +1,43 @@ 'use strict'; const { - getOptions, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getCLIOptions, + getEmbedderOptions: getEmbedderOptionsFromBinding, } = internalBinding('options'); let warnOnAllowUnauthorized = true; let optionsMap; let aliasesMap; +let embedderOptions; -// getOptions() would serialize the option values from C++ land. +// getCLIOptions() would serialize the option values from C++ land. // It would error if the values are queried before bootstrap is // complete so that we don't accidentally include runtime-dependent // states into a runtime-independent snapshot. -function getOptionsFromBinding() { +function getCLIOptionsFromBinding() { if (!optionsMap) { - ({ options: optionsMap } = getOptions()); + ({ options: optionsMap } = getCLIOptions()); } return optionsMap; } function getAliasesFromBinding() { if (!aliasesMap) { - ({ aliases: aliasesMap } = getOptions()); + ({ aliases: aliasesMap } = getCLIOptions()); } return aliasesMap; } +function getEmbedderOptions() { + if (!embedderOptions) { + embedderOptions = getEmbedderOptionsFromBinding(); + } + return embedderOptions; +} + function getOptionValue(optionName) { - const options = getOptionsFromBinding(); + const options = getCLIOptionsFromBinding(); if (optionName.startsWith('--no-')) { const option = options.get('--' + optionName.slice(5)); return option && !option.value; @@ -54,13 +61,12 @@ function getAllowUnauthorized() { module.exports = { get options() { - return getOptionsFromBinding(); + return getCLIOptionsFromBinding(); }, get aliases() { return getAliasesFromBinding(); }, getOptionValue, getAllowUnauthorized, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getEmbedderOptions }; diff --git a/src/node_options.cc b/src/node_options.cc index c7f153f1d87817..acec2d72515d1d 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -915,7 +915,7 @@ std::string GetBashCompletion() { // Return a map containing all the options and their metadata as well // as the aliases -void GetOptions(const FunctionCallbackInfo& args) { +void GetCLIOptions(const FunctionCallbackInfo& args) { Mutex::ScopedLock lock(per_process::cli_options_mutex); Environment* env = Environment::GetCurrent(args); if (!env->has_run_bootstrapping_code()) { @@ -1056,13 +1056,38 @@ void GetOptions(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } +void GetEmbedderOptions(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!env->has_run_bootstrapping_code()) { + // No code because this is an assertion. + return env->ThrowError( + "Should not query options before bootstrapping is done"); + } + Isolate* isolate = args.GetIsolate(); + Local context = env->context(); + Local ret = Object::New(isolate); + + if (ret->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), + Boolean::New(isolate, env->should_not_register_esm_loader())) + .IsNothing()) return; + + if (ret->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"), + Boolean::New(isolate, env->no_global_search_paths())) + .IsNothing()) return; + + args.GetReturnValue().Set(ret); +} + void Initialize(Local target, Local unused, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); - env->SetMethodNoSideEffect(target, "getOptions", GetOptions); + env->SetMethodNoSideEffect(target, "getCLIOptions", GetCLIOptions); + env->SetMethodNoSideEffect(target, "getEmbedderOptions", GetEmbedderOptions); Local env_settings = Object::New(isolate); NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvironment); @@ -1072,18 +1097,6 @@ void Initialize(Local target, context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings) .Check(); - target - ->Set(context, - FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), - Boolean::New(isolate, env->should_not_register_esm_loader())) - .Check(); - - target - ->Set(context, - FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"), - Boolean::New(isolate, env->no_global_search_paths())) - .Check(); - Local types = Object::New(isolate); NODE_DEFINE_CONSTANT(types, kNoOp); NODE_DEFINE_CONSTANT(types, kV8Option); diff --git a/src/node_options.h b/src/node_options.h index e5744333140a02..5cf2bb442fad40 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -461,7 +461,7 @@ class OptionsParser { template friend class OptionsParser; - friend void GetOptions(const v8::FunctionCallbackInfo& args); + friend void GetCLIOptions(const v8::FunctionCallbackInfo& args); friend std::string GetBashCompletion(); };