src: bootstrap prepare stack trace callback in shadow realm

Bootstrap per-realm callbacks like `prepare_stack_trace_callback` in the ShadowRealm. This enables stack trace decoration in the ShadowRealm. PR-URL: #47107 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Nov 10, 2023 · d35f870 · d35f870
1 parent d668738
commit d35f870
Show file tree

Hide file tree

Showing 18 changed files with 79 additions and 69 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -89,7 +89,7 @@
 /doc/api/module.md @nodejs/modules @nodejs/loaders
 /doc/api/modules.md @nodejs/modules @nodejs/loaders
 /doc/api/packages.md @nodejs/modules @nodejs/loaders
-/lib/internal/bootstrap/loaders.js @nodejs/modules @nodejs/loaders
+/lib/internal/bootstrap/realm.js @nodejs/modules @nodejs/loaders
 /lib/internal/modules/* @nodejs/modules @nodejs/loaders
 /lib/internal/process/esm_loader.js @nodejs/modules @nodejs/loaders
 /lib/internal/process/execution.js @nodejs/modules @nodejs/loaders

diff --git a/lib/assert.js b/lib/assert.js
@@ -65,7 +65,7 @@ const { openSync, closeSync, readSync } = require('fs');
 const { inspect } = require('internal/util/inspect');
 const { isPromise, isRegExp } = require('internal/util/types');
 const { EOL } = require('internal/constants');
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const { isError } = require('internal/util');
 
 const errorCache = new SafeMap();

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -1,6 +1,6 @@
 // Hello, and welcome to hacking node.js!
 //
-// This file is invoked by `Realm::BootstrapNode()` in `src/node_realm.cc`,
+// This file is invoked by `Realm::BootstrapRealm()` in `src/node_realm.cc`,
 // and is responsible for setting up Node.js core before main scripts
 // under `lib/internal/main/` are executed.
 //
@@ -32,9 +32,10 @@
 //   `DOMException` class.
 // - `lib/internal/per_context/messageport.js`: JS-side components of the
 //   `MessagePort` implementation.
-// - `lib/internal/bootstrap/loaders.js`: this sets up internal binding and
+// - `lib/internal/bootstrap/realm.js`: this sets up internal binding and
 //   module loaders, including `process.binding()`, `process._linkedBinding()`,
-//   `internalBinding()` and `BuiltinModule`.
+//   `internalBinding()` and `BuiltinModule`, and per-realm internal states
+//   and bindings, including `prepare_stack_trace_callback`.
 //
 // The initialization done in this script is included in both the main thread
 // and the worker threads. After this, further initialization is done based
@@ -52,8 +53,6 @@
 // passed by `BuiltinLoader::CompileAndCall()`.
 /* global process, require, internalBinding, primordials */
 
-setupPrepareStackTrace();
-
 const {
   FunctionPrototypeCall,
   JSONParse,
@@ -364,25 +363,6 @@ process.emitWarning = emitWarning;
   // Note: only after this point are the timers effective
 }
 
-function setupPrepareStackTrace() {
-  const {
-    setEnhanceStackForFatalException,
-    setPrepareStackTraceCallback,
-  } = internalBinding('errors');
-  const {
-    prepareStackTrace,
-    fatalExceptionStackEnhancers: {
-      beforeInspector,
-      afterInspector,
-    },
-  } = require('internal/errors');
-  // Tell our PrepareStackTraceCallback passed to the V8 API
-  // to call prepareStackTrace().
-  setPrepareStackTraceCallback(prepareStackTrace);
-  // Set the function used to enhance the error stack for printing
-  setEnhanceStackForFatalException(beforeInspector, afterInspector);
-}
-
 function setupProcessObject() {
   const EventEmitter = require('events');
   const origProcProto = ObjectGetPrototypeOf(process);

diff --git a/lib/internal/bootstrap/loaders.js → lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/loaders.js → lib/internal/bootstrap/realm.js
@@ -1,3 +1,8 @@
+// This file is executed in every realm that is created by Node.js, including
+// the context of main thread, worker threads, and ShadowRealms.
+// Only per-realm internal states and bindings should be bootstrapped in this
+// file and no globals should be exposed to the user code.
+//
 // This file creates the internal module & binding loaders used by built-in
 // modules. In contrast, user land modules are loaded using
 // lib/internal/modules/cjs/loader.js (CommonJS Modules) or
@@ -30,7 +35,7 @@
 //   so they can be loaded faster without the cost of I/O. This class makes the
 //   lib/internal/*, deps/internal/* modules and internalBinding() available by
 //   default to core modules, and lets the core modules require itself via
-//   require('internal/bootstrap/loaders') even when this file is not written in
+//   require('internal/bootstrap/realm') even when this file is not written in
 //   CommonJS style.
 //
 // Other objects:
@@ -181,7 +186,7 @@ let internalBinding;
   };
 }
 
-const loaderId = 'internal/bootstrap/loaders';
+const selfId = 'internal/bootstrap/realm';
 const {
   builtinIds,
   compileFunction,
@@ -238,7 +243,7 @@ class BuiltinModule {
   static exposeInternals() {
     for (const { 0: id, 1: mod } of BuiltinModule.map) {
       // Do not expose this to user land even with --expose-internals.
-      if (id !== loaderId) {
+      if (id !== selfId) {
         mod.canBeRequiredByUsers = true;
       }
     }
@@ -357,7 +362,7 @@ const loaderExports = {
 };
 
 function requireBuiltin(id) {
-  if (id === loaderId) {
+  if (id === selfId) {
     return loaderExports;
   }
 
@@ -379,5 +384,27 @@ function requireWithFallbackInDeps(request) {
   return requireBuiltin(request);
 }
 
+function setupPrepareStackTrace() {
+  const {
+    setEnhanceStackForFatalException,
+    setPrepareStackTraceCallback,
+  } = internalBinding('errors');
+  const {
+    prepareStackTrace,
+    fatalExceptionStackEnhancers: {
+      beforeInspector,
+      afterInspector,
+    },
+  } = requireBuiltin('internal/errors');
+  // Tell our PrepareStackTraceCallback passed to the V8 API
+  // to call prepareStackTrace().
+  setPrepareStackTraceCallback(prepareStackTrace);
+  // Set the function used to enhance the error stack for printing
+  setEnhanceStackForFatalException(beforeInspector, afterInspector);
+}
+
 // Store the internal loaders in C++.
 setInternalLoaders(internalBinding, requireBuiltin);
+
+// Setup per-realm bindings.
+setupPrepareStackTrace();
diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js
@@ -12,7 +12,7 @@ const {
 } = primordials;
 
 const binding = internalBinding('mksnapshot');
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   compileSerializeMain,
 } = binding;

diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -76,7 +76,7 @@ module.exports = {
   initializeCJS,
 };
 
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   maybeCacheSourceMap,
 } = require('internal/source_map/source_map_cache');

diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -190,7 +190,7 @@ class Hooks {
           filename: '<preload>',
         },
       );
-      const { BuiltinModule } = require('internal/bootstrap/loaders');
+      const { BuiltinModule } = require('internal/bootstrap/realm');
       // We only allow replacing the importMetaInitializer during preload;
       // after preload is finished, we disable the ability to replace it.
       //

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -24,7 +24,7 @@ const {
   StringPrototypeStartsWith,
 } = primordials;
 const internalFS = require('internal/fs/utils');
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   realpathSync,
   statSync,

diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js
@@ -18,7 +18,7 @@ const {
   ERR_MANIFEST_DEPENDENCY_MISSING,
   ERR_UNKNOWN_BUILTIN_MODULE,
 } = require('internal/errors').codes;
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 
 const { validateString } = require('internal/validators');
 const path = require('path');

diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -339,7 +339,7 @@ function initializeReport() {
 function setupDebugEnv() {
   require('internal/util/debuglog').initializeDebugEnv(process.env.NODE_DEBUG);
   if (getOptionValue('--expose-internals')) {
-    require('internal/bootstrap/loaders').BuiltinModule.exposeInternals();
+    require('internal/bootstrap/realm').BuiltinModule.exposeInternals();
   }
 }
 

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
@@ -151,7 +151,7 @@ const {
 
 const assert = require('internal/assert');
 
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   validateObject,
   validateString,

diff --git a/lib/repl.js b/lib/repl.js
@@ -97,7 +97,7 @@ const {
   globalThis,
 } = primordials;
 
-const { BuiltinModule } = require('internal/bootstrap/loaders');
+const { BuiltinModule } = require('internal/bootstrap/realm');
 const {
   makeRequireFunction,
   addBuiltinLibsToObject,

diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -65,7 +65,18 @@ MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
   if (env == nullptr) {
     return exception->ToString(context).FromMaybe(Local<Value>());
   }
-  Local<Function> prepare = env->prepare_stack_trace_callback();
+  Realm* realm = Realm::GetCurrent(context);
+  Local<Function> prepare;
+  if (realm != nullptr) {
+    // If we are in a Realm, call the realm specific prepareStackTrace callback
+    // to avoid passing the JS objects (the exception and trace) across the
+    // realm boundary with the `Error.prepareStackTrace` override.
+    prepare = realm->prepare_stack_trace_callback();
+  } else {
+    // The context is created with ContextifyContext, call the principal
+    // realm's prepareStackTrace callback.
+    prepare = env->principal_realm()->prepare_stack_trace_callback();
+  }
   if (prepare.IsEmpty()) {
     return exception->ToString(context).FromMaybe(Local<Value>());
   }

diff --git a/src/node_builtins.cc b/src/node_builtins.cc
@@ -363,9 +363,9 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompile(Local<Context> context,
   std::vector<Local<String>> parameters;
   Isolate* isolate = context->GetIsolate();
   // Detects parameters of the scripts based on module ids.
-  // internal/bootstrap/loaders: process, getLinkedBinding,
-  //                             getInternalBinding, primordials
-  if (strcmp(id, "internal/bootstrap/loaders") == 0) {
+  // internal/bootstrap/realm: process, getLinkedBinding,
+  //                           getInternalBinding, primordials
+  if (strcmp(id, "internal/bootstrap/realm") == 0) {
     parameters = {
         FIXED_ONE_BYTE_STRING(isolate, "process"),
         FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"),
@@ -427,9 +427,9 @@ MaybeLocal<Value> BuiltinLoader::CompileAndCall(Local<Context> context,
   // BuiltinLoader::LookupAndCompile().
   std::vector<Local<Value>> arguments;
   // Detects parameters of the scripts based on module ids.
-  // internal/bootstrap/loaders: process, getLinkedBinding,
-  //                             getInternalBinding, primordials
-  if (strcmp(id, "internal/bootstrap/loaders") == 0) {
+  // internal/bootstrap/realm: process, getLinkedBinding,
+  //                           getInternalBinding, primordials
+  if (strcmp(id, "internal/bootstrap/realm") == 0) {
     Local<Value> get_linked_binding;
     Local<Value> get_internal_binding;
     if (!NewFunctionTemplate(isolate, binding::GetLinkedBinding)

diff --git a/src/node_errors.cc b/src/node_errors.cc
@@ -954,9 +954,9 @@ void PerIsolateMessageListener(Local<Message> message, Local<Value> error) {
 }
 
 void SetPrepareStackTraceCallback(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
+  Realm* realm = Realm::GetCurrent(args);
   CHECK(args[0]->IsFunction());
-  env->set_prepare_stack_trace_callback(args[0].As<Function>());
+  realm->set_prepare_stack_trace_callback(args[0].As<Function>());
 }
 
 static void SetSourceMapsEnabled(const FunctionCallbackInfo<Value>& args) {
@@ -981,11 +981,11 @@ static void SetMaybeCacheGeneratedSourceMap(
 
 static void SetEnhanceStackForFatalException(
     const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
+  Realm* realm = Realm::GetCurrent(args);
   CHECK(args[0]->IsFunction());
   CHECK(args[1]->IsFunction());
-  env->set_enhance_fatal_stack_before_inspector(args[0].As<Function>());
-  env->set_enhance_fatal_stack_after_inspector(args[1].As<Function>());
+  realm->set_enhance_fatal_stack_before_inspector(args[0].As<Function>());
+  realm->set_enhance_fatal_stack_after_inspector(args[1].As<Function>());
 }
 
 // Side effect-free stringification that will never throw exceptions.

diff --git a/src/node_realm.cc b/src/node_realm.cc
@@ -183,18 +183,14 @@ MaybeLocal<Value> Realm::ExecuteBootstrapper(const char* id) {
 }
 
 MaybeLocal<Value> Realm::BootstrapNode() {
-  EscapableHandleScope scope(isolate_);
-
-  MaybeLocal<Value> result = ExecuteBootstrapper("internal/bootstrap/node");
+  HandleScope scope(isolate_);
 
-  if (result.IsEmpty()) {
+  if (ExecuteBootstrapper("internal/bootstrap/node").IsEmpty()) {
     return MaybeLocal<Value>();
   }
 
   if (!env_->no_browser_globals()) {
-    result = ExecuteBootstrapper("internal/bootstrap/browser");
-
-    if (result.IsEmpty()) {
+    if (ExecuteBootstrapper("internal/bootstrap/browser").IsEmpty()) {
       return MaybeLocal<Value>();
     }
   }
@@ -203,19 +199,15 @@ MaybeLocal<Value> Realm::BootstrapNode() {
   auto thread_switch_id =
       env_->is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
                              : "internal/bootstrap/switches/is_not_main_thread";
-  result = ExecuteBootstrapper(thread_switch_id);
-
-  if (result.IsEmpty()) {
+  if (ExecuteBootstrapper(thread_switch_id).IsEmpty()) {
     return MaybeLocal<Value>();
   }
 
   auto process_state_switch_id =
       env_->owns_process_state()
           ? "internal/bootstrap/switches/does_own_process_state"
           : "internal/bootstrap/switches/does_not_own_process_state";
-  result = ExecuteBootstrapper(process_state_switch_id);
-
-  if (result.IsEmpty()) {
+  if (ExecuteBootstrapper(process_state_switch_id).IsEmpty()) {
     return MaybeLocal<Value>();
   }
 
@@ -227,7 +219,7 @@ MaybeLocal<Value> Realm::BootstrapNode() {
     return MaybeLocal<Value>();
   }
 
-  return scope.EscapeMaybe(result);
+  return v8::True(isolate_);
 }
 
 MaybeLocal<Value> Realm::RunBootstrapping() {
@@ -236,7 +228,7 @@ MaybeLocal<Value> Realm::RunBootstrapping() {
   CHECK(!has_run_bootstrapping_code());
 
   Local<Value> result;
-  if (!ExecuteBootstrapper("internal/bootstrap/loaders").ToLocal(&result)) {
+  if (!ExecuteBootstrapper("internal/bootstrap/realm").ToLocal(&result)) {
     return MaybeLocal<Value>();
   }
 

diff --git a/test/es-module/test-loaders-hidden-from-users.js b/test/es-module/test-loaders-hidden-from-users.js
@@ -7,16 +7,16 @@ const assert = require('assert');
 
 assert.throws(
   () => {
-    require('internal/bootstrap/loaders');
+    require('internal/bootstrap/realm');
   }, {
     code: 'MODULE_NOT_FOUND',
-    message: /Cannot find module 'internal\/bootstrap\/loaders'/
+    message: /Cannot find module 'internal\/bootstrap\/realm'/
   }
 );
 
 assert.throws(
   () => {
-    const source = 'module.exports = require("internal/bootstrap/loaders")';
+    const source = 'module.exports = require("internal/bootstrap/realm")';
     const { internalBinding } = require('internal/test/binding');
     internalBinding('natives').owo = source;
     require('owo');

diff --git a/test/parallel/test-inspector-inspect-brk-node.js b/test/parallel/test-inspector-inspect-brk-node.js
@@ -16,7 +16,7 @@ async function runTest() {
   await session.waitForNotification((notification) => {
     // The main assertion here is that we do hit the loader script first.
     return notification.method === 'Debugger.scriptParsed' &&
-           notification.params.url === 'node:internal/bootstrap/loaders';
+           notification.params.url === 'node:internal/bootstrap/realm';
   });
 
   await session.waitForNotification('Debugger.paused');