From 277c70a98f16a70e45c5c32db7ed62cf54c534ca Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 21 Mar 2025 15:37:34 +0000 Subject: [PATCH 1/2] src: ensure primordials are initialized exactly once PR-URL: https://github.com/nodejs/node/pull/57519 Reviewed-By: Antoine du Hamel Reviewed-By: James M Snell --- src/api/environment.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 09f6e930e26aef..6a0ae8aedc2d3f 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -761,7 +761,7 @@ Maybe InitializeMainContextForSnapshot(Local context) { if (InitializeBaseContextForSnapshot(context).IsNothing()) { return Nothing(); } - return InitializePrimordials(context); + return JustVoid(); } Maybe InitializePrimordials(Local context) { @@ -770,13 +770,17 @@ Maybe InitializePrimordials(Local context) { Context::Scope context_scope(context); Local exports; + if (!GetPerContextExports(context).ToLocal(&exports)) { + return Nothing(); + } Local primordials_string = FIXED_ONE_BYTE_STRING(isolate, "primordials"); + // Ensure that `InitializePrimordials` is called exactly once on a given + // context. + CHECK(!exports->Has(context, primordials_string).FromJust()); - // Create primordials first and make it available to per-context scripts. Local primordials = Object::New(isolate); - if (primordials->SetPrototype(context, Null(isolate)).IsNothing() || - !GetPerContextExports(context).ToLocal(&exports) || + if (primordials->SetPrototype(context, Null(isolate)).IsNothing() || exports->Set(context, primordials_string, primordials).IsNothing()) { return Nothing(); } From 1e5c8a2eb4c969ada3836fa41f531c6f3d392371 Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Sun, 6 Apr 2025 17:17:37 +0930 Subject: [PATCH 2/2] src: initialize privateSymbols for per_context PR-URL: https://github.com/nodejs/node/pull/57479 Reviewed-By: Chengzhong Wu --- src/api/environment.cc | 48 +++++++++++++++++++++++++++++++++++++----- src/node_builtins.cc | 1 + src/node_internals.h | 8 +++++-- src/node_messaging.cc | 15 +++++++------ src/node_realm.cc | 2 +- 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 6a0ae8aedc2d3f..df5fb942aa893c 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -1,4 +1,5 @@ #include +#include "env_properties.h" #include "node.h" #include "node_builtins.h" #include "node_context_data.h" @@ -602,7 +603,8 @@ std::unique_ptr MultiIsolatePlatform::Create( page_allocator); } -MaybeLocal GetPerContextExports(Local context) { +MaybeLocal GetPerContextExports(Local context, + IsolateData* isolate_data) { Isolate* isolate = context->GetIsolate(); EscapableHandleScope handle_scope(isolate); @@ -616,10 +618,14 @@ MaybeLocal GetPerContextExports(Local context) { if (existing_value->IsObject()) return handle_scope.Escape(existing_value.As()); + // To initialize the per-context binding exports, a non-nullptr isolate_data + // is needed + CHECK(isolate_data); Local exports = Object::New(isolate); if (context->Global()->SetPrivate(context, key, exports).IsNothing() || - InitializePrimordials(context).IsNothing()) + InitializePrimordials(context, isolate_data).IsNothing()) { return MaybeLocal(); + } return handle_scope.Escape(exports); } @@ -764,7 +770,32 @@ Maybe InitializeMainContextForSnapshot(Local context) { return JustVoid(); } -Maybe InitializePrimordials(Local context) { +MaybeLocal InitializePrivateSymbols(Local context, + IsolateData* isolate_data) { + CHECK(isolate_data); + Isolate* isolate = context->GetIsolate(); + EscapableHandleScope scope(isolate); + Context::Scope context_scope(context); + + Local private_symbols = ObjectTemplate::New(isolate); + Local 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->SetPrototype(context, Null(isolate)) + .IsNothing()) { + return MaybeLocal(); + } + + return scope.Escape(private_symbols_object); +} + +Maybe InitializePrimordials(Local context, + IsolateData* isolate_data) { // Run per-context JS files. Isolate* isolate = context->GetIsolate(); Context::Scope context_scope(context); @@ -780,11 +811,17 @@ Maybe InitializePrimordials(Local context) { CHECK(!exports->Has(context, primordials_string).FromJust()); Local primordials = Object::New(isolate); - if (primordials->SetPrototype(context, Null(isolate)).IsNothing() || + if (primordials->SetPrototype(context, Null(isolate)).IsNothing() || exports->Set(context, primordials_string, primordials).IsNothing()) { return Nothing(); } + Local private_symbols; + if (!InitializePrivateSymbols(context, isolate_data) + .ToLocal(&private_symbols)) { + return Nothing(); + } + static const char* context_files[] = {"internal/per_context/primordials", "internal/per_context/domexception", "internal/per_context/messageport", @@ -800,7 +837,8 @@ Maybe InitializePrimordials(Local context) { builtin_loader.SetEagerCompile(); for (const char** module = context_files; *module != nullptr; module++) { - Local arguments[] = {exports, primordials}; + Local arguments[] = {exports, primordials, private_symbols}; + if (builtin_loader .CompileAndCall( context, *module, arraysize(arguments), arguments, nullptr) diff --git a/src/node_builtins.cc b/src/node_builtins.cc index 578121e8e1ffbe..defb657a62a031 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -412,6 +412,7 @@ MaybeLocal BuiltinLoader::LookupAndCompile(Local 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 index 382df89a2312f7..275534285ec28f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -113,7 +113,10 @@ std::string GetHumanReadableProcessName(); v8::Maybe InitializeBaseContextForSnapshot( v8::Local context); v8::Maybe InitializeContextRuntime(v8::Local context); -v8::Maybe InitializePrimordials(v8::Local context); +v8::Maybe InitializePrimordials(v8::Local context, + IsolateData* isolate_data); +v8::MaybeLocal InitializePrivateSymbols( + v8::Local 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 StartExecution(Environment* env, StartExecutionCallback cb = nullptr); -v8::MaybeLocal GetPerContextExports(v8::Local context); +v8::MaybeLocal GetPerContextExports( + v8::Local context, IsolateData* isolate_data = nullptr); void MarkBootstrapComplete(const v8::FunctionCallbackInfo& args); class InitializationResultImpl final : public InitializationResult { diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 02b5f9239d37a1..6b3fb5d7da515b 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -251,14 +251,16 @@ void Message::AdoptSharedValueConveyor(SharedValueConveyor&& conveyor) { namespace { -MaybeLocal GetEmitMessageFunction(Local context) { +MaybeLocal GetEmitMessageFunction(Local context, + IsolateData* isolate_data) { Isolate* isolate = context->GetIsolate(); Local per_context_bindings; Local emit_message_val; - if (!GetPerContextExports(context).ToLocal(&per_context_bindings) || - !per_context_bindings->Get(context, - FIXED_ONE_BYTE_STRING(isolate, "emitMessage")) - .ToLocal(&emit_message_val)) { + if (!GetPerContextExports(context, isolate_data) + .ToLocal(&per_context_bindings) || + !per_context_bindings + ->Get(context, FIXED_ONE_BYTE_STRING(isolate, "emitMessage")) + .ToLocal(&emit_message_val)) { return MaybeLocal(); } CHECK(emit_message_val->IsFunction()); @@ -687,7 +689,8 @@ MessagePort::MessagePort(Environment* env, } Local emit_message_fn; - if (!GetEmitMessageFunction(context).ToLocal(&emit_message_fn)) + if (!GetEmitMessageFunction(context, env->isolate_data()) + .ToLocal(&emit_message_fn)) return; emit_message_fn_.Reset(env->isolate(), emit_message_fn); diff --git a/src/node_realm.cc b/src/node_realm.cc index 6262ed8cde59f2..e87c6e2da49368 100644 --- a/src/node_realm.cc +++ b/src/node_realm.cc @@ -45,7 +45,7 @@ void Realm::CreateProperties() { // Store primordials setup by the per-context script in the environment. Local per_context_bindings = - GetPerContextExports(ctx).ToLocalChecked(); + GetPerContextExports(ctx, env_->isolate_data()).ToLocalChecked(); Local primordials = per_context_bindings->Get(ctx, env_->primordials_string()) .ToLocalChecked();