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: #40357 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
nodejs · Oct 16, 2021 · 38aa7cc · 38aa7cc
1 parent 6baea14
commit 38aa7cc
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 30 deletions.
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);
+
+  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<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();
 };