Skip to content

Commit 4c9445f

Browse files
committed
src,bootstrap: expose isolate_data even before env
In some cases, we need `IsolateData` even before env, 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`.
1 parent 59f00d7 commit 4c9445f

File tree

10 files changed

+84
-27
lines changed

10 files changed

+84
-27
lines changed

src/api/environment.cc

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <cstdlib>
2+
#include "env_properties.h"
23
#include "node.h"
34
#include "node_builtins.h"
45
#include "node_context_data.h"
@@ -599,7 +600,8 @@ std::unique_ptr<MultiIsolatePlatform> MultiIsolatePlatform::Create(
599600
page_allocator);
600601
}
601602

602-
MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
603+
MaybeLocal<Object> GetPerContextExports(Local<Context> context,
604+
IsolateData* isolate_data) {
603605
Isolate* isolate = context->GetIsolate();
604606
EscapableHandleScope handle_scope(isolate);
605607

@@ -615,20 +617,22 @@ MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
615617

616618
Local<Object> exports = Object::New(isolate);
617619
if (context->Global()->SetPrivate(context, key, exports).IsNothing() ||
618-
InitializePrimordials(context).IsNothing())
620+
InitializePrimordials(context, isolate_data).IsNothing()) {
619621
return MaybeLocal<Object>();
622+
}
620623
return handle_scope.Escape(exports);
621624
}
622625

623626
// Any initialization logic should be performed in
624627
// InitializeContext, because embedders don't necessarily
625628
// call NewContext and so they will experience breakages.
626629
Local<Context> NewContext(Isolate* isolate,
630+
IsolateData* isolate_data,
627631
Local<ObjectTemplate> object_template) {
628632
auto context = Context::New(isolate, nullptr, object_template);
629633
if (context.IsEmpty()) return context;
630634

631-
if (InitializeContext(context).IsNothing()) {
635+
if (InitializeContext(context, isolate_data).IsNothing()) {
632636
return Local<Context>();
633637
}
634638

@@ -745,7 +749,8 @@ Maybe<void> InitializeBaseContextForSnapshot(Local<Context> context) {
745749
return JustVoid();
746750
}
747751

748-
Maybe<void> InitializeMainContextForSnapshot(Local<Context> context) {
752+
Maybe<void> InitializeMainContextForSnapshot(Local<Context> context,
753+
IsolateData* isolate_data) {
749754
Isolate* isolate = context->GetIsolate();
750755
HandleScope handle_scope(isolate);
751756

@@ -758,10 +763,34 @@ Maybe<void> InitializeMainContextForSnapshot(Local<Context> context) {
758763
if (InitializeBaseContextForSnapshot(context).IsNothing()) {
759764
return Nothing<void>();
760765
}
761-
return InitializePrimordials(context);
766+
return InitializePrimordials(context, isolate_data);
767+
}
768+
769+
Local<Object> InitializePrivateSymbols(Local<Context> context,
770+
IsolateData* isolate_data) {
771+
Isolate* isolate = context->GetIsolate();
772+
EscapableHandleScope scope(isolate);
773+
Context::Scope context_scope(context);
774+
775+
Local<ObjectTemplate> private_symbols = ObjectTemplate::New(isolate);
776+
Local<Object> private_symbols_object;
777+
#define V(PropertyName, _) \
778+
private_symbols->Set(isolate, #PropertyName, isolate_data->PropertyName());
779+
780+
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
781+
#undef V
782+
783+
if (!private_symbols->NewInstance(context).ToLocal(&private_symbols_object) ||
784+
private_symbols_object->SetPrototypeV2(context, Null(isolate))
785+
.IsNothing()) {
786+
return Local<Object>();
787+
}
788+
789+
return scope.Escape(private_symbols_object);
762790
}
763791

764-
Maybe<void> InitializePrimordials(Local<Context> context) {
792+
Maybe<void> InitializePrimordials(Local<Context> context,
793+
IsolateData* isolate_data) {
765794
// Run per-context JS files.
766795
Isolate* isolate = context->GetIsolate();
767796
Context::Scope context_scope(context);
@@ -772,12 +801,18 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
772801

773802
// Create primordials first and make it available to per-context scripts.
774803
Local<Object> primordials = Object::New(isolate);
804+
Local<Object> private_symbols;
805+
775806
if (primordials->SetPrototypeV2(context, Null(isolate)).IsNothing() ||
776-
!GetPerContextExports(context).ToLocal(&exports) ||
807+
!GetPerContextExports(context, isolate_data).ToLocal(&exports) ||
777808
exports->Set(context, primordials_string, primordials).IsNothing()) {
778809
return Nothing<void>();
779810
}
780811

812+
if (isolate_data != nullptr) {
813+
private_symbols = InitializePrivateSymbols(context, isolate_data);
814+
}
815+
781816
static const char* context_files[] = {"internal/per_context/primordials",
782817
"internal/per_context/domexception",
783818
"internal/per_context/messageport",
@@ -793,7 +828,12 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
793828
builtin_loader.SetEagerCompile();
794829

795830
for (const char** module = context_files; *module != nullptr; module++) {
796-
Local<Value> arguments[] = {exports, primordials};
831+
Local<Value> arguments[3];
832+
arguments[0] = exports;
833+
arguments[1] = primordials;
834+
arguments[2] = private_symbols.IsEmpty() ? Local<Value>(Undefined(isolate))
835+
: Local<Value>(private_symbols);
836+
797837
if (builtin_loader
798838
.CompileAndCall(
799839
context, *module, arraysize(arguments), arguments, nullptr)
@@ -807,8 +847,9 @@ Maybe<void> InitializePrimordials(Local<Context> context) {
807847
}
808848

809849
// This initializes the main context (i.e. vm contexts are not included).
810-
Maybe<bool> InitializeContext(Local<Context> context) {
811-
if (InitializeMainContextForSnapshot(context).IsNothing()) {
850+
Maybe<bool> InitializeContext(Local<Context> context,
851+
IsolateData* isolate_data) {
852+
if (InitializeMainContextForSnapshot(context, isolate_data).IsNothing()) {
812853
return Nothing<bool>();
813854
}
814855

src/node.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,12 +590,14 @@ NODE_EXTERN v8::Isolate* NewIsolate(
590590
// Creates a new context with Node.js-specific tweaks.
591591
NODE_EXTERN v8::Local<v8::Context> NewContext(
592592
v8::Isolate* isolate,
593+
IsolateData* isolate_data = nullptr,
593594
v8::Local<v8::ObjectTemplate> object_template =
594595
v8::Local<v8::ObjectTemplate>());
595596

596597
// Runs Node.js-specific tweaks on an already constructed context
597598
// Return value indicates success of operation
598-
NODE_EXTERN v8::Maybe<bool> InitializeContext(v8::Local<v8::Context> context);
599+
NODE_EXTERN v8::Maybe<bool> InitializeContext(
600+
v8::Local<v8::Context> context, IsolateData* isolate_data = nullptr);
599601

600602
// If `platform` is passed, it will be used to register new Worker instances.
601603
// It can be `nullptr`, in which case creating new Workers inside of

src/node_builtins.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompile(Local<Context> context,
402402
parameters = {
403403
FIXED_ONE_BYTE_STRING(isolate, "exports"),
404404
FIXED_ONE_BYTE_STRING(isolate, "primordials"),
405+
FIXED_ONE_BYTE_STRING(isolate, "privateSymbols"),
405406
};
406407
} else if (strncmp(id, "internal/main/", strlen("internal/main/")) == 0 ||
407408
strncmp(id,

src/node_internals.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,10 @@ std::string GetHumanReadableProcessName();
113113
v8::Maybe<void> InitializeBaseContextForSnapshot(
114114
v8::Local<v8::Context> context);
115115
v8::Maybe<void> InitializeContextRuntime(v8::Local<v8::Context> context);
116-
v8::Maybe<void> InitializePrimordials(v8::Local<v8::Context> context);
116+
v8::Maybe<void> InitializePrimordials(v8::Local<v8::Context> context,
117+
IsolateData* isolate_data);
118+
v8::Local<v8::Object> InitializePrivateSymbols(v8::Local<v8::Context> context,
119+
IsolateData* isolate_data);
117120

118121
class NodeArrayBufferAllocator : public ArrayBufferAllocator {
119122
public:
@@ -340,7 +343,8 @@ v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
340343
// was provided by the embedder.
341344
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
342345
StartExecutionCallback cb = nullptr);
343-
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);
346+
v8::MaybeLocal<v8::Object> GetPerContextExports(
347+
v8::Local<v8::Context> context, IsolateData* isolate_data = nullptr);
344348
void MarkBootstrapComplete(const v8::FunctionCallbackInfo<v8::Value>& args);
345349

346350
class InitializationResultImpl final : public InitializationResult {

src/node_main_instance.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ NodeMainInstance::CreateMainEnvironment(ExitCode* exit_code) {
140140
crypto::InitCryptoOnce(isolate_);
141141
#endif // HAVE_OPENSSL
142142
} else {
143-
context = NewContext(isolate_);
143+
context = NewContext(isolate_, isolate_data_.get());
144144
CHECK(!context.IsEmpty());
145145
Context::Scope context_scope(context);
146146
env.reset(

src/node_snapshotable.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
11031103
// The Node.js-specific context with primodials, can be used by workers
11041104
// TODO(joyeecheung): investigate if this can be used by vm contexts
11051105
// without breaking compatibility.
1106-
Local<Context> base_context = NewContext(isolate);
1106+
Local<Context> base_context = NewContext(isolate, setup->isolate_data());
11071107
if (base_context.IsEmpty()) {
11081108
return ExitCode::kBootstrapFailure;
11091109
}

src/node_worker.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,15 @@ void Worker::Run() {
338338
context = Context::FromSnapshot(isolate_,
339339
SnapshotData::kNodeBaseContextIndex)
340340
.ToLocalChecked();
341+
341342
if (!context.IsEmpty() &&
342343
!InitializeContextRuntime(context).IsJust()) {
343344
context = Local<Context>();
344345
}
345346
} else {
346347
Debug(
347348
this, "Worker %llu builds context from scratch\n", thread_id_.id);
348-
context = NewContext(isolate_);
349+
context = NewContext(isolate_, data.isolate_data_.get());
349350
}
350351
if (context.IsEmpty()) {
351352
// TODO(joyeecheung): maybe this should be kBootstrapFailure instead?

test/cctest/node_test_fixture.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ class EnvironmentTestFixture : public NodeTestFixture {
155155
node::EnvironmentFlags::Flags flags =
156156
node::EnvironmentFlags::kDefaultFlags) {
157157
auto isolate = handle_scope.GetIsolate();
158-
context_ = node::NewContext(isolate);
158+
context_ =
159+
node::NewContext(isolate, EnvironmentTestFixture::isolate_data_);
159160
CHECK(!context_.IsEmpty());
160161
context_->Enter();
161162

@@ -176,6 +177,10 @@ class EnvironmentTestFixture : public NodeTestFixture {
176177
context_->Exit();
177178
}
178179

180+
inline node::IsolateData* isolate_data() const {
181+
return EnvironmentTestFixture::isolate_data_;
182+
}
183+
179184
node::Environment* operator*() const {
180185
return environment_;
181186
}

test/cctest/test_environment.cc

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
478478
node::MultiIsolatePlatform* platform;
479479
int32_t extracted_value = -1;
480480
uv_async_t thread_stopped_async;
481+
Env* env;
481482
};
482483

483484
ChildEnvironmentData data;
@@ -496,6 +497,7 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
496497
});
497498
CHECK_EQ(err, 0);
498499
data.thread_stopped_async.data = &thread_stopped;
500+
data.env = &env;
499501

500502
uv_thread_t thread;
501503
err = uv_thread_create(&thread, [](void* arg) {
@@ -512,7 +514,8 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) {
512514
v8::Isolate::Scope isolate_scope(isolate);
513515
v8::HandleScope handle_scope(isolate);
514516

515-
v8::Local<v8::Context> context = node::NewContext(isolate);
517+
v8::Local<v8::Context> context =
518+
node::NewContext(isolate, data->env->isolate_data());
516519
CHECK(!context.IsEmpty());
517520
v8::Context::Scope context_scope(context);
518521

@@ -636,17 +639,17 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
636639
v8::Isolate::Scope isolate_scope(isolate);
637640
v8::HandleScope handle_scope(isolate);
638641

639-
auto context = node::NewContext(isolate);
640-
CHECK(!context.IsEmpty());
641-
v8::Context::Scope context_scope(context);
642-
643642
std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
644643
isolate_data{node::CreateIsolateData(isolate,
645644
&current_loop,
646645
platform.get()),
647646
node::FreeIsolateData};
648647
CHECK(isolate_data);
649648

649+
auto context = node::NewContext(isolate);
650+
CHECK(!context.IsEmpty());
651+
v8::Context::Scope context_scope(context);
652+
650653
std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
651654
environment{node::CreateEnvironment(isolate_data.get(),
652655
context,
@@ -762,7 +765,7 @@ TEST_F(EnvironmentTest, RequestInterruptAtExit) {
762765
const v8::HandleScope handle_scope(isolate_);
763766
const Argv argv;
764767

765-
Local<Context> context = node::NewContext(isolate_);
768+
Local<Context> context = node::NewContext(isolate_, isolate_data_);
766769
CHECK(!context.IsEmpty());
767770
context->Enter();
768771

test/cctest/test_platform.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,17 @@ TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
8282
v8::Isolate::Scope isolate_scope(isolate);
8383
v8::HandleScope handle_scope(isolate);
8484

85-
auto context = node::NewContext(isolate);
86-
CHECK(!context.IsEmpty());
87-
v8::Context::Scope context_scope(context);
88-
8985
std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
9086
isolate_data{node::CreateIsolateData(isolate,
9187
&current_loop,
9288
platform.get()),
9389
node::FreeIsolateData};
9490
CHECK(isolate_data);
9591

92+
auto context = node::NewContext(isolate, isolate_data.get());
93+
CHECK(!context.IsEmpty());
94+
v8::Context::Scope context_scope(context);
95+
9696
std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
9797
environment{node::CreateEnvironment(isolate_data.get(),
9898
context,

0 commit comments

Comments
 (0)