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.
nodejs · joyeecheung · Oct 7, 2021 · Oct 11, 2021 · Oct 7, 2021 · dc394bcdaff95286488e3de00cd52b463f5ea48e
commit dc394bcdaff95286488e3de00cd52b463f5ea48e
diff --git 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
@@ -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
@@ -920,7 +920,7 @@ std::string GetBashCompletion() {
 
 // Return a map containing all the options and their metadata as well
 // as the aliases
-void GetOptions(const FunctionCallbackInfo<Value>& args) {
+void GetCLIOptions(const FunctionCallbackInfo<Value>& args) {
   Mutex::ScopedLock lock(per_process::cli_options_mutex);
   Environment* env = Environment::GetCurrent(args);
   if (!env->has_run_bootstrapping_code()) {
@@ -1061,13 +1061,38 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(ret);
 }
 
+void GetEmbedderOptions(const FunctionCallbackInfo<Value>& 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> context = env->context();
+  Local<Object> ret = Object::New(isolate);
+
+  ret->Set(context,
+           FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
+           Boolean::New(isolate, env->should_not_register_esm_loader()))
+      .Check();
+
+  ret->Set(context,
+           FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"),
+           Boolean::New(isolate, env->no_global_search_paths()))
+      .Check();
+
+  args.GetReturnValue().Set(ret);
+}
+
 void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> 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<Object> env_settings = Object::New(isolate);
   NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvironment);
@@ -1077,18 +1102,6 @@ void Initialize(Local<Object> 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<Object> 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
@@ -462,7 +462,7 @@ class OptionsParser {
   template <typename OtherOptions>
   friend class OptionsParser;
 
-  friend void GetOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
+  friend void GetCLIOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
   friend std::string GetBashCompletion();
 };