src,bootstrap: expose isolate_data even before env

jazelly · jazelly · commit f4643b92f59a · 2025-03-15T16:00:57.000+10:30
In some cases, we need `IsolateData` even before env, e.g. for JS files
living in `per_context`. This commit provides a safe way without
breaking current context initialization process, to pass down
`IsolateData` to `InitializePrimordials`.
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -1,4 +1,5 @@
 #include <cstdlib>
+#include "env_properties.h"
 #include "node.h"
 #include "node_builtins.h"
 #include "node_context_data.h"
@@ -599,7 +600,8 @@ std::unique_ptr<MultiIsolatePlatform> MultiIsolatePlatform::Create(
                                         page_allocator);
 }
 
-MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
+MaybeLocal<Object> GetPerContextExports(Local<Context> context,
+                                        IsolateData* isolate_data) {
   Isolate* isolate = context->GetIsolate();
   EscapableHandleScope handle_scope(isolate);
 
@@ -615,20 +617,22 @@ MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
 
   Local<Object> exports = Object::New(isolate);
   if (context->Global()->SetPrivate(context, key, exports).IsNothing() ||
-      InitializePrimordials(context).IsNothing())
+      InitializePrimordials(context, isolate_data).IsNothing()) {
     return MaybeLocal<Object>();
+  }
   return handle_scope.Escape(exports);
 }
 
 // Any initialization logic should be performed in
 // InitializeContext, because embedders don't necessarily
 // call NewContext and so they will experience breakages.
 Local<Context> NewContext(Isolate* isolate,
+                          IsolateData* isolate_data,
                           Local<ObjectTemplate> object_template) {
   auto context = Context::New(isolate, nullptr, object_template);
   if (context.IsEmpty()) return context;
 
-  if (InitializeContext(context).IsNothing()) {
+  if (InitializeContext(context, isolate_data).IsNothing()) {
     return Local<Context>();
   }
 
@@ -745,7 +749,8 @@ Maybe<void> InitializeBaseContextForSnapshot(Local<Context> context) {
   return JustVoid();
 }
 
-Maybe<void> InitializeMainContextForSnapshot(Local<Context> context) {
+Maybe<void> InitializeMainContextForSnapshot(Local<Context> context,
+                                             IsolateData* isolate_data) {
   Isolate* isolate = context->GetIsolate();
   HandleScope handle_scope(isolate);
 
@@ -758,10 +763,34 @@ Maybe<void> InitializeMainContextForSnapshot(Local<Context> context) {
   if (InitializeBaseContextForSnapshot(context).IsNothing()) {
     return Nothing<void>();
   }
-  return InitializePrimordials(context);
+  return InitializePrimordials(context, isolate_data);
+}
+
+Local<Object> InitializePrivateSymbols(Local<Context> context,
+                                       IsolateData* isolate_data) {
+  Isolate* isolate = context->GetIsolate();
+  EscapableHandleScope scope(isolate);
+  Context::Scope context_scope(context);
+
+  Local<ObjectTemplate> private_symbols = ObjectTemplate::New(isolate);
+  Local<Object> private_symbols_object;
+#define V(PropertyName, _)                                                     \
+  private_symbols->Set(isolate, #PropertyName, isolate_data->PropertyName());
+
+  PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
+#undef V
+
+  if (!private_symbols->NewInstance(context).ToLocal(&private_symbols_object) ||
+      private_symbols_object->SetPrototypeV2(context, Null(isolate))
+          .IsNothing()) {
+    return Local<Object>();
+  }
+
+  return scope.Escape(private_symbols_object);
 }
 
-Maybe<void> InitializePrimordials(Local<Context> context) {
+Maybe<void> InitializePrimordials(Local<Context> context,
+                                  IsolateData* isolate_data) {
   // Run per-context JS files.
   Isolate* isolate = context->GetIsolate();
   Context::Scope context_scope(context);
@@ -772,12 +801,18 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
 
   // Create primordials first and make it available to per-context scripts.
   Local<Object> primordials = Object::New(isolate);
+  Local<Object> private_symbols;
+
   if (primordials->SetPrototypeV2(context, Null(isolate)).IsNothing() ||
-      !GetPerContextExports(context).ToLocal(&exports) ||
+      !GetPerContextExports(context, isolate_data).ToLocal(&exports) ||
       exports->Set(context, primordials_string, primordials).IsNothing()) {
     return Nothing<void>();
   }
 
+  if (isolate_data != nullptr) {
+    private_symbols = InitializePrivateSymbols(context, isolate_data);
+  }
+
   static const char* context_files[] = {"internal/per_context/primordials",
                                         "internal/per_context/domexception",
                                         "internal/per_context/messageport",
@@ -793,7 +828,12 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
   builtin_loader.SetEagerCompile();
 
   for (const char** module = context_files; *module != nullptr; module++) {
-    Local<Value> arguments[] = {exports, primordials};
+    Local<Value> arguments[3];
+    arguments[0] = exports;
+    arguments[1] = primordials;
+    arguments[2] = private_symbols.IsEmpty() ? Local<Value>(Undefined(isolate))
+                                             : Local<Value>(private_symbols);
+
     if (builtin_loader
             .CompileAndCall(
                 context, *module, arraysize(arguments), arguments, nullptr)
@@ -807,8 +847,9 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
 }
 
 // This initializes the main context (i.e. vm contexts are not included).
-Maybe<bool> InitializeContext(Local<Context> context) {
-  if (InitializeMainContextForSnapshot(context).IsNothing()) {
+Maybe<bool> InitializeContext(Local<Context> context,
+                              IsolateData* isolate_data) {
+  if (InitializeMainContextForSnapshot(context, isolate_data).IsNothing()) {
     return Nothing<bool>();
   }
 
diff --git a/src/node.h b/src/node.h
@@ -590,12 +590,14 @@ NODE_EXTERN v8::Isolate* NewIsolate(
 // Creates a new context with Node.js-specific tweaks.
 NODE_EXTERN v8::Local<v8::Context> NewContext(
     v8::Isolate* isolate,
+    IsolateData* isolate_data = nullptr,
     v8::Local<v8::ObjectTemplate> object_template =
         v8::Local<v8::ObjectTemplate>());
 
 // Runs Node.js-specific tweaks on an already constructed context
 // Return value indicates success of operation
-NODE_EXTERN v8::Maybe<bool> InitializeContext(v8::Local<v8::Context> context);
+NODE_EXTERN v8::Maybe<bool> InitializeContext(
+    v8::Local<v8::Context> context, IsolateData* isolate_data = nullptr);
 
 // If `platform` is passed, it will be used to register new Worker instances.
 // It can be `nullptr`, in which case creating new Workers inside of
diff --git a/src/node_builtins.cc b/src/node_builtins.cc
@@ -402,6 +402,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompile(Local<Context> context,
     parameters = {
         FIXED_ONE_BYTE_STRING(isolate, "exports"),
         FIXED_ONE_BYTE_STRING(isolate, "primordials"),
+        FIXED_ONE_BYTE_STRING(isolate, "privateSymbols"),
     };
   } else if (strncmp(id, "internal/main/", strlen("internal/main/")) == 0 ||
              strncmp(id,
diff --git a/src/node_internals.h b/src/node_internals.h
@@ -113,7 +113,10 @@ std::string GetHumanReadableProcessName();
 v8::Maybe<void> InitializeBaseContextForSnapshot(
     v8::Local<v8::Context> context);
 v8::Maybe<void> InitializeContextRuntime(v8::Local<v8::Context> context);
-v8::Maybe<void> InitializePrimordials(v8::Local<v8::Context> context);
+v8::Maybe<void> InitializePrimordials(v8::Local<v8::Context> context,
+                                      IsolateData* isolate_data);
+v8::Local<v8::Object> InitializePrivateSymbols(v8::Local<v8::Context> context,
+                                               IsolateData* isolate_data);
 
 class NodeArrayBufferAllocator : public ArrayBufferAllocator {
  public:
@@ -340,7 +343,8 @@ v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
 // was provided by the embedder.
 v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
                                          StartExecutionCallback cb = nullptr);
-v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
+v8::MaybeLocal<v8::Object> GetPerContextExports(
+    v8::Local<v8::Context> context, IsolateData* isolate_data = nullptr);
 void MarkBootstrapComplete(const v8::FunctionCallbackInfo<v8::Value>& args);
 
 class InitializationResultImpl final : public InitializationResult {
diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -140,7 +140,7 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* exit_code) {
     crypto::InitCryptoOnce(isolate_);
 #endif  // HAVE_OPENSSL
   } else {
-    context = NewContext(isolate_);
+    context = NewContext(isolate_, isolate_data_.get());
     CHECK(!context.IsEmpty());
     Context::Scope context_scope(context);
     env.reset(
diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -1103,7 +1103,7 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
     // The Node.js-specific context with primodials, can be used by workers
     // TODO(joyeecheung): investigate if this can be used by vm contexts
     // without breaking compatibility.
-    Local<Context> base_context = NewContext(isolate);
+    Local<Context> base_context = NewContext(isolate, setup->isolate_data());
     if (base_context.IsEmpty()) {
       return ExitCode::kBootstrapFailure;
     }
diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -345,7 +345,7 @@ void Worker::Run() {
         } else {
           Debug(
               this, "Worker %llu builds context from scratch\n", thread_id_.id);
-          context = NewContext(isolate_);
+          context = NewContext(isolate_, data.isolate_data_.get());
         }
         if (context.IsEmpty()) {
           // TODO(joyeecheung): maybe this should be kBootstrapFailure instead?
diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h
@@ -155,7 +155,8 @@ class EnvironmentTestFixture : public NodeTestFixture {
         node::EnvironmentFlags::Flags flags =
             node::EnvironmentFlags::kDefaultFlags) {
       auto isolate = handle_scope.GetIsolate();
-      context_ = node::NewContext(isolate);
+      context_ =
+          node::NewContext(isolate, EnvironmentTestFixture::isolate_data_);
       CHECK(!context_.IsEmpty());
       context_->Enter();
 
@@ -176,6 +177,10 @@ class EnvironmentTestFixture : public NodeTestFixture {
       context_->Exit();
     }
 
+    inline node::IsolateData* isolate_data() const {
+      return EnvironmentTestFixture::isolate_data_;
+    }
+
     node::Environment* operator*() const {
       return environment_;
     }
diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
@@ -478,6 +478,7 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
     node::MultiIsolatePlatform* platform;
     int32_t extracted_value = -1;
     uv_async_t thread_stopped_async;
+    Env* env;
   };
 
   ChildEnvironmentData data;
@@ -496,6 +497,7 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
       });
   CHECK_EQ(err, 0);
   data.thread_stopped_async.data = &thread_stopped;
+  data.env = &env;
 
   uv_thread_t thread;
   err = uv_thread_create(&thread, [](void* arg) {
@@ -512,7 +514,8 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
       v8::Isolate::Scope isolate_scope(isolate);
       v8::HandleScope handle_scope(isolate);
 
-      v8::Local<v8::Context> context = node::NewContext(isolate);
+      v8::Local<v8::Context> context =
+          node::NewContext(isolate, data->env->isolate_data());
       CHECK(!context.IsEmpty());
       v8::Context::Scope context_scope(context);
 
@@ -636,17 +639,17 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
     v8::Isolate::Scope isolate_scope(isolate);
     v8::HandleScope handle_scope(isolate);
 
-    auto context = node::NewContext(isolate);
-    CHECK(!context.IsEmpty());
-    v8::Context::Scope context_scope(context);
-
     std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
       isolate_data{node::CreateIsolateData(isolate,
                                            &current_loop,
                                            platform.get()),
                    node::FreeIsolateData};
     CHECK(isolate_data);
 
+    auto context = node::NewContext(isolate);
+    CHECK(!context.IsEmpty());
+    v8::Context::Scope context_scope(context);
+
     std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
       environment{node::CreateEnvironment(isolate_data.get(),
                                           context,
@@ -762,7 +765,7 @@ TEST_F(EnvironmentTest, RequestInterruptAtExit) {
   const v8::HandleScope handle_scope(isolate_);
   const Argv argv;
 
-  Local<Context> context = node::NewContext(isolate_);
+  Local<Context> context = node::NewContext(isolate_, isolate_data_);
   CHECK(!context.IsEmpty());
   context->Enter();
 
diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc
@@ -82,17 +82,17 @@ TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
     v8::Isolate::Scope isolate_scope(isolate);
     v8::HandleScope handle_scope(isolate);
 
-    auto context = node::NewContext(isolate);
-    CHECK(!context.IsEmpty());
-    v8::Context::Scope context_scope(context);
-
     std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
       isolate_data{node::CreateIsolateData(isolate,
                                            &current_loop,
                                            platform.get()),
                    node::FreeIsolateData};
     CHECK(isolate_data);
 
+    auto context = node::NewContext(isolate, isolate_data.get());
+    CHECK(!context.IsEmpty());
+    v8::Context::Scope context_scope(context);
+
     std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
       environment{node::CreateEnvironment(isolate_data.get(),
                                           context,

Original file line number	Diff line number	Diff line change
`@@ -1103,7 +1103,7 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,`
`1103`	`1103`	`// The Node.js-specific context with primodials, can be used by workers`
`1104`	`1104`	`// TODO(joyeecheung): investigate if this can be used by vm contexts`
`1105`	`1105`	`// without breaking compatibility.`
`1106`		`- Local<Context> base_context = NewContext(isolate);`
	`1106`	`+ Local<Context> base_context = NewContext(isolate, setup->isolate_data());`
`1107`	`1107`	`if (base_context.IsEmpty()) {`
`1108`	`1108`	`return ExitCode::kBootstrapFailure;`
`1109`	`1109`	`}`
Original file line number	Diff line number	Diff line change
`@@ -345,7 +345,7 @@ void Worker::Run() {`
`345`	`345`	`} else {`
`346`	`346`	`Debug(`
`347`	`347`	`this, "Worker %llu builds context from scratch\n", thread_id_.id);`
`348`		`- context = NewContext(isolate_);`
	`348`	`+ context = NewContext(isolate_, data.isolate_data_.get());`
`349`	`349`	`}`
`350`	`350`	`if (context.IsEmpty()) {`
`351`	`351`	`// TODO(joyeecheung): maybe this should be kBootstrapFailure instead?`