src: initialize PerIsolateData eagerly

PR-URL: nodejs/node#21983 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
electron · Oct 5, 2018 · 5331c9d · 5331c9d
1 parent c5042a2
commit 5331c9d
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 16 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -42,7 +42,7 @@ IsolateData::IsolateData(Isolate* isolate,
     zero_fill_field_(zero_fill_field),
     platform_(platform) {
   if (platform_ != nullptr)
-    platform_->RegisterIsolate(this, event_loop);
+    platform_->RegisterIsolate(isolate_, event_loop);
 
   options_.reset(new PerIsolateOptions(*per_process_opts->per_isolate));
 
@@ -95,7 +95,7 @@ IsolateData::IsolateData(Isolate* isolate,
 
 IsolateData::~IsolateData() {
   if (platform_ != nullptr)
-    platform_->UnregisterIsolate(this);
+    platform_->UnregisterIsolate(isolate_);
 }
 
 

diff --git a/src/node.cc b/src/node.cc
@@ -2984,17 +2984,22 @@ bool AllowWasmCodeGenerationCallback(
   return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
 }
 
-Isolate* NewIsolate(ArrayBufferAllocator* allocator) {
+Isolate* NewIsolate(ArrayBufferAllocator* allocator, uv_loop_t* event_loop) {
   Isolate::CreateParams params;
   params.array_buffer_allocator = allocator;
 #ifdef NODE_ENABLE_VTUNE_PROFILING
   params.code_event_handler = vTune::GetVtuneCodeEventHandler();
 #endif
 
-  Isolate* isolate = Isolate::New(params);
+  Isolate* isolate = Isolate::Allocate();
   if (isolate == nullptr)
     return nullptr;
 
+  // Register the isolate on the platform before the isolate gets initialized,
+  // so that the isolate can access the platform during initialization.
+  v8_platform.Platform()->RegisterIsolate(isolate, event_loop);
+  Isolate::Initialize(isolate, params);
+
   isolate->AddMessageListener(OnMessage);
   isolate->SetAbortOnUncaughtExceptionCallback(ShouldAbortOnUncaughtException);
   isolate->SetMicrotasksPolicy(MicrotasksPolicy::kExplicit);
@@ -3009,7 +3014,7 @@ inline int Start(uv_loop_t* event_loop,
                  const std::vector<std::string>& exec_args) {
   std::unique_ptr<ArrayBufferAllocator, decltype(&FreeArrayBufferAllocator)>
       allocator(CreateArrayBufferAllocator(), &FreeArrayBufferAllocator);
-  Isolate* const isolate = NewIsolate(allocator.get());
+  Isolate* const isolate = NewIsolate(allocator.get(), event_loop);
   if (isolate == nullptr)
     return 12;  // Signal internal error.
 
@@ -3047,6 +3052,7 @@ inline int Start(uv_loop_t* event_loop,
   }
 
   isolate->Dispose();
+  v8_platform.Platform()->UnregisterIsolate(isolate);
 
   return exit_code;
 }

diff --git a/src/node.h b/src/node.h
@@ -254,13 +254,14 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
   virtual void CancelPendingDelayedTasks(v8::Isolate* isolate) = 0;
 
   // These will be called by the `IsolateData` creation/destruction functions.
-  virtual void RegisterIsolate(IsolateData* isolate_data,
+  virtual void RegisterIsolate(v8::Isolate* isolate,
                                struct uv_loop_s* loop) = 0;
-  virtual void UnregisterIsolate(IsolateData* isolate_data) = 0;
+  virtual void UnregisterIsolate(v8::Isolate* isolate) = 0;
 };
 
 // Creates a new isolate with Node.js-specific settings.
-NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator);
+NODE_EXTERN v8::Isolate* NewIsolate(ArrayBufferAllocator* allocator,
+                                    struct uv_loop_s* event_loop);
 
 // Creates a new context with Node.js-specific tweaks.
 NODE_EXTERN v8::Local<v8::Context> NewContext(

diff --git a/src/node_platform.cc b/src/node_platform.cc
@@ -260,8 +260,7 @@ NodePlatform::NodePlatform(int thread_pool_size,
       std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
 }
 
-void NodePlatform::RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) {
-  Isolate* isolate = isolate_data->isolate();
+void NodePlatform::RegisterIsolate(Isolate* isolate, uv_loop_t* loop) {
   Mutex::ScopedLock lock(per_isolate_mutex_);
   std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
   if (existing) {
@@ -273,8 +272,7 @@ void NodePlatform::RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) {
   }
 }
 
-void NodePlatform::UnregisterIsolate(IsolateData* isolate_data) {
-  Isolate* isolate = isolate_data->isolate();
+void NodePlatform::UnregisterIsolate(Isolate* isolate) {
   Mutex::ScopedLock lock(per_isolate_mutex_);
   std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
   CHECK(existing);

diff --git a/src/node_platform.h b/src/node_platform.h
@@ -141,8 +141,8 @@ class NodePlatform : public MultiIsolatePlatform {
   v8::TracingController* GetTracingController() override;
   bool FlushForegroundTasks(v8::Isolate* isolate) override;
 
-  void RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) override;
-  void UnregisterIsolate(IsolateData* isolate_data) override;
+  void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
+  void UnregisterIsolate(v8::Isolate* isolate) override;
 
   std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner(
       v8::Isolate* isolate) override;

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -68,9 +68,9 @@ Worker::Worker(Environment* env, Local<Object> wrap)
 
   array_buffer_allocator_.reset(CreateArrayBufferAllocator());
 
-  isolate_ = NewIsolate(array_buffer_allocator_.get());
-  CHECK_NE(isolate_, nullptr);
   CHECK_EQ(uv_loop_init(&loop_), 0);
+  isolate_ = NewIsolate(array_buffer_allocator_.get(), &loop_);
+  CHECK_NE(isolate_, nullptr);
 
   {
     // Enter an environment capable of executing code in the child Isolate
@@ -262,6 +262,7 @@ void Worker::DisposeIsolate() {
   platform->CancelPendingDelayedTasks(isolate_);
 
   isolate_data_.reset();
+  platform->UnregisterIsolate(isolate_);
 
   isolate_->Dispose();
   isolate_ = nullptr;

diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h
@@ -90,10 +90,13 @@ class NodeTestFixture : public ::testing::Test {
                                      &node::FreeArrayBufferAllocator);
     isolate_ = NewIsolate(allocator.get());
     CHECK_NE(isolate_, nullptr);
+    platform->RegisterIsolate(isolate_, &current_loop);
+    v8::Isolate::Initialize(isolate_, params);
   }
 
   virtual void TearDown() {
     isolate_->Dispose();
+    platform->UnregisterIsolate(isolate_);
     isolate_ = nullptr;
   }
 };