src: add abstract IsolatePlatformDelegate

Adds a new abstract class for module authors and embedders to register
arbitrary isolates with `node::MultiIsolatePlatform`.

PR-URL: https://github.com/nodejs/node/pull/30324
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

src/node.h

@@ -262,6 +262,12 @@ class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
 NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
 NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);
 
+class NODE_EXTERN IsolatePlatformDelegate {
+ public:
+  virtual std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner() = 0;
+  virtual bool IdleTasksEnabled() = 0;
+};
+
 class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
  public:
   ~MultiIsolatePlatform() override = default;
@@ -283,6 +289,12 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
   // This function may only be called once per `Isolate`.
   virtual void RegisterIsolate(v8::Isolate* isolate,
                                struct uv_loop_s* loop) = 0;
+  // This method can be used when an application handles task scheduling on its
+  // own through `IsolatePlatformDelegate`. Upon registering an isolate with
+  // this overload any other method in this class with the exception of
+  // `UnregisterIsolate` *must not* be used on that isolate.
+  virtual void RegisterIsolate(v8::Isolate* isolate,
+                               IsolatePlatformDelegate* delegate) = 0;
 
   // This function may only be called once per `Isolate`, and discard any
   // pending delayed tasks scheduled for that isolate.

src/node_platform.cc

@@ -230,6 +230,11 @@ PerIsolatePlatformData::PerIsolatePlatformData(
   uv_unref(reinterpret_cast<uv_handle_t*>(flush_tasks_));
 }
 
+std::shared_ptr<v8::TaskRunner>
+PerIsolatePlatformData::GetForegroundTaskRunner() {
+  return shared_from_this();
+}
+
 void PerIsolatePlatformData::FlushTasks(uv_async_t* handle) {
   auto platform_data = static_cast<PerIsolatePlatformData*>(handle->data);
   platform_data->FlushForegroundTasksInternal();
@@ -267,7 +272,7 @@ void PerIsolatePlatformData::PostNonNestableDelayedTask(
 }
 
 PerIsolatePlatformData::~PerIsolatePlatformData() {
-  Shutdown();
+  CHECK(!flush_tasks_);
 }
 
 void PerIsolatePlatformData::AddShutdownCallback(void (*callback)(void*),
@@ -325,18 +330,32 @@ NodePlatform::NodePlatform(int thread_pool_size,
 
 void NodePlatform::RegisterIsolate(Isolate* isolate, uv_loop_t* loop) {
   Mutex::ScopedLock lock(per_isolate_mutex_);
-  std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
-  CHECK(!existing);
-  per_isolate_[isolate] =
-      std::make_shared<PerIsolatePlatformData>(isolate, loop);
+  auto delegate = std::make_shared<PerIsolatePlatformData>(isolate, loop);
+  IsolatePlatformDelegate* ptr = delegate.get();
+  auto insertion = per_isolate_.emplace(
+    isolate,
+    std::make_pair(ptr, std::move(delegate)));
+  CHECK(insertion.second);
+}
+
+void NodePlatform::RegisterIsolate(Isolate* isolate,
+                                   IsolatePlatformDelegate* delegate) {
+  Mutex::ScopedLock lock(per_isolate_mutex_);
+  auto insertion = per_isolate_.emplace(
+    isolate,
+    std::make_pair(delegate, std::shared_ptr<PerIsolatePlatformData>{}));
+  CHECK(insertion.second);
 }
 
 void NodePlatform::UnregisterIsolate(Isolate* isolate) {
   Mutex::ScopedLock lock(per_isolate_mutex_);
-  std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
-  CHECK(existing);
-  existing->Shutdown();
-  per_isolate_.erase(isolate);
+  auto existing_it = per_isolate_.find(isolate);
+  CHECK_NE(existing_it, per_isolate_.end());
+  auto& existing = existing_it->second;
+  if (existing.second) {
+    existing.second->Shutdown();
+  }
+  per_isolate_.erase(existing_it);
 }
 
 void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
@@ -344,11 +363,11 @@ void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
   Mutex::ScopedLock lock(per_isolate_mutex_);
   auto it = per_isolate_.find(isolate);
   if (it == per_isolate_.end()) {
-    CHECK(it->second);
     cb(data);
     return;
   }
-  it->second->AddShutdownCallback(cb, data);
+  CHECK(it->second.second);
+  it->second.second->AddShutdownCallback(cb, data);
 }
 
 void NodePlatform::Shutdown() {
@@ -394,7 +413,7 @@ void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
 }
 
 void NodePlatform::DrainTasks(Isolate* isolate) {
-  std::shared_ptr<PerIsolatePlatformData> per_isolate = ForIsolate(isolate);
+  std::shared_ptr<PerIsolatePlatformData> per_isolate = ForNodeIsolate(isolate);
 
   do {
     // Worker tasks aren't associated with an Isolate.
@@ -452,23 +471,32 @@ void NodePlatform::CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
 }
 
 
-std::shared_ptr<PerIsolatePlatformData>
-NodePlatform::ForIsolate(Isolate* isolate) {
+IsolatePlatformDelegate* NodePlatform::ForIsolate(Isolate* isolate) {
   Mutex::ScopedLock lock(per_isolate_mutex_);
-  std::shared_ptr<PerIsolatePlatformData> data = per_isolate_[isolate];
-  CHECK(data);
-  return data;
+  auto data = per_isolate_[isolate];
+  CHECK_NOT_NULL(data.first);
+  return data.first;
+}
+
+std::shared_ptr<PerIsolatePlatformData>
+NodePlatform::ForNodeIsolate(Isolate* isolate) {
+  Mutex::ScopedLock lock(per_isolate_mutex_);
+  auto data = per_isolate_[isolate];
+  CHECK(data.second);
+  return data.second;
 }
 
 bool NodePlatform::FlushForegroundTasks(Isolate* isolate) {
-  return ForIsolate(isolate)->FlushForegroundTasksInternal();
+  return ForNodeIsolate(isolate)->FlushForegroundTasksInternal();
 }
 
-bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { return false; }
+bool NodePlatform::IdleTasksEnabled(Isolate* isolate) {
+  return ForIsolate(isolate)->IdleTasksEnabled();
+}
 
 std::shared_ptr<v8::TaskRunner>
 NodePlatform::GetForegroundTaskRunner(Isolate* isolate) {
-  return ForIsolate(isolate);
+  return ForIsolate(isolate)->GetForegroundTaskRunner();
 }
 
 double NodePlatform::MonotonicallyIncreasingTime() {

src/node_platform.h

@@ -52,12 +52,14 @@ struct DelayedTask {
 
 // This acts as the foreground task runner for a given Isolate.
 class PerIsolatePlatformData :
+    public IsolatePlatformDelegate,
     public v8::TaskRunner,
     public std::enable_shared_from_this<PerIsolatePlatformData> {
  public:
   PerIsolatePlatformData(v8::Isolate* isolate, uv_loop_t* loop);
   ~PerIsolatePlatformData() override;
 
+  std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner() override;
   void PostTask(std::unique_ptr<v8::Task> task) override;
   void PostIdleTask(std::unique_ptr<v8::IdleTask> task) override;
   void PostDelayedTask(std::unique_ptr<v8::Task> task,
@@ -162,6 +164,9 @@ class NodePlatform : public MultiIsolatePlatform {
   bool FlushForegroundTasks(v8::Isolate* isolate) override;
 
   void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
+  void RegisterIsolate(v8::Isolate* isolate,
+                       IsolatePlatformDelegate* delegate) override;
+
   void UnregisterIsolate(v8::Isolate* isolate) override;
   void AddIsolateFinishedCallback(v8::Isolate* isolate,
                                   void (*callback)(void*), void* data) override;
@@ -170,11 +175,13 @@ class NodePlatform : public MultiIsolatePlatform {
       v8::Isolate* isolate) override;
 
  private:
-  std::shared_ptr<PerIsolatePlatformData> ForIsolate(v8::Isolate* isolate);
+  IsolatePlatformDelegate* ForIsolate(v8::Isolate* isolate);
+  std::shared_ptr<PerIsolatePlatformData> ForNodeIsolate(v8::Isolate* isolate);
 
   Mutex per_isolate_mutex_;
-  std::unordered_map<v8::Isolate*,
-                     std::shared_ptr<PerIsolatePlatformData>> per_isolate_;
+  using DelegatePair = std::pair<
+    IsolatePlatformDelegate*, std::shared_ptr<PerIsolatePlatformData>>;
+  std::unordered_map<v8::Isolate*, DelegatePair> per_isolate_;
 
   node::tracing::TracingController* tracing_controller_;
   std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;

test/cctest/node_test_fixture.cc

@@ -1,7 +1,7 @@
 #include "node_test_fixture.h"
 
-ArrayBufferUniquePtr NodeTestFixture::allocator{nullptr, nullptr};
-uv_loop_t NodeTestFixture::current_loop;
-NodePlatformUniquePtr NodeTestFixture::platform;
-TracingAgentUniquePtr NodeTestFixture::tracing_agent;
-bool NodeTestFixture::node_initialized = false;
+ArrayBufferUniquePtr NodeZeroIsolateTestFixture::allocator{nullptr, nullptr};
+uv_loop_t NodeZeroIsolateTestFixture::current_loop;
+NodePlatformUniquePtr NodeZeroIsolateTestFixture::platform;
+TracingAgentUniquePtr NodeZeroIsolateTestFixture::tracing_agent;
+bool NodeZeroIsolateTestFixture::node_initialized = false;

test/cctest/node_test_fixture.h

@@ -60,14 +60,13 @@ using ArrayBufferUniquePtr = std::unique_ptr<node::ArrayBufferAllocator,
 using TracingAgentUniquePtr = std::unique_ptr<node::tracing::Agent>;
 using NodePlatformUniquePtr = std::unique_ptr<node::NodePlatform>;
 
-class NodeTestFixture : public ::testing::Test {
+class NodeZeroIsolateTestFixture : public ::testing::Test {
  protected:
   static ArrayBufferUniquePtr allocator;
   static TracingAgentUniquePtr tracing_agent;
   static NodePlatformUniquePtr platform;
   static uv_loop_t current_loop;
   static bool node_initialized;
-  v8::Isolate* isolate_;
 
   static void SetUpTestCase() {
     if (!node_initialized) {
@@ -99,8 +98,18 @@ class NodeTestFixture : public ::testing::Test {
   void SetUp() override {
     allocator = ArrayBufferUniquePtr(node::CreateArrayBufferAllocator(),
                                      &node::FreeArrayBufferAllocator);
+  }
+};
+
+
+class NodeTestFixture : public NodeZeroIsolateTestFixture {
+ protected:
+  v8::Isolate* isolate_;
+
+  void SetUp() override {
+    NodeZeroIsolateTestFixture::SetUp();
     isolate_ = NewIsolate(allocator.get(), &current_loop);
-    CHECK_NE(isolate_, nullptr);
+    CHECK_NOT_NULL(isolate_);
     isolate_->Enter();
   }
 
@@ -110,6 +119,7 @@ class NodeTestFixture : public ::testing::Test {
     isolate_->Dispose();
     platform->UnregisterIsolate(isolate_);
     isolate_ = nullptr;
+    NodeZeroIsolateTestFixture::TearDown();
   }
 };
 

test/cctest/test_platform.cc

@@ -57,3 +57,50 @@ TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) {
   EXPECT_EQ(3, run_count);
   EXPECT_FALSE(platform->FlushForegroundTasks(isolate_));
 }
+
+// Tests the registration of an abstract `IsolatePlatformDelegate` instance as
+// opposed to the more common `uv_loop_s*` version of `RegisterIsolate`.
+TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
+  // Allocate isolate
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = allocator.get();
+  auto isolate = v8::Isolate::Allocate();
+  CHECK_NOT_NULL(isolate);
+
+  // Register *first*, then initialize
+  auto delegate = std::make_shared<node::PerIsolatePlatformData>(
+    isolate,
+    &current_loop);
+  platform->RegisterIsolate(isolate, delegate.get());
+  v8::Isolate::Initialize(isolate, create_params);
+
+  // Try creating Context + IsolateData + Environment
+  {
+    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);
+
+    std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
+      environment{node::CreateEnvironment(isolate_data.get(),
+                                          context,
+                                          0, nullptr,
+                                          0, nullptr),
+                  node::FreeEnvironment};
+    CHECK(environment);
+  }
+
+  // Graceful shutdown
+  delegate->Shutdown();
+  isolate->Dispose();
+  platform->UnregisterIsolate(isolate);
+}