Skip to content

Commit c712fb7

Browse files
laverdetaddaleax
laverdet
authored and
addaleax
committed
src: add abstract IsolatePlatformDelegate
Adds a new abstract class for module authors and embedders to register arbitrary isolates with `node::MultiIsolatePlatform`. PR-URL: #30324 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent c63af4f commit c712fb7

File tree

6 files changed

+134
-30
lines changed

6 files changed

+134
-30
lines changed

‎src/node.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,12 @@ class NODE_EXTERN ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
262262
NODE_EXTERN ArrayBufferAllocator* CreateArrayBufferAllocator();
263263
NODE_EXTERN void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator);
264264

265+
class NODE_EXTERN IsolatePlatformDelegate {
266+
public:
267+
virtual std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner() = 0;
268+
virtual bool IdleTasksEnabled() = 0;
269+
};
270+
265271
class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
266272
public:
267273
~MultiIsolatePlatform() override = default;
@@ -283,6 +289,12 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
283289
// This function may only be called once per `Isolate`.
284290
virtual void RegisterIsolate(v8::Isolate* isolate,
285291
struct uv_loop_s* loop) = 0;
292+
// This method can be used when an application handles task scheduling on its
293+
// own through `IsolatePlatformDelegate`. Upon registering an isolate with
294+
// this overload any other method in this class with the exception of
295+
// `UnregisterIsolate` *must not* be used on that isolate.
296+
virtual void RegisterIsolate(v8::Isolate* isolate,
297+
IsolatePlatformDelegate* delegate) = 0;
286298

287299
// This function may only be called once per `Isolate`, and discard any
288300
// pending delayed tasks scheduled for that isolate.

‎src/node_platform.cc

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,11 @@ PerIsolatePlatformData::PerIsolatePlatformData(
230230
uv_unref(reinterpret_cast<uv_handle_t*>(flush_tasks_));
231231
}
232232

233+
std::shared_ptr<v8::TaskRunner>
234+
PerIsolatePlatformData::GetForegroundTaskRunner() {
235+
return shared_from_this();
236+
}
237+
233238
void PerIsolatePlatformData::FlushTasks(uv_async_t* handle) {
234239
auto platform_data = static_cast<PerIsolatePlatformData*>(handle->data);
235240
platform_data->FlushForegroundTasksInternal();
@@ -267,7 +272,7 @@ void PerIsolatePlatformData::PostNonNestableDelayedTask(
267272
}
268273

269274
PerIsolatePlatformData::~PerIsolatePlatformData() {
270-
Shutdown();
275+
CHECK(!flush_tasks_);
271276
}
272277

273278
void PerIsolatePlatformData::AddShutdownCallback(void (*callback)(void*),
@@ -325,30 +330,44 @@ NodePlatform::NodePlatform(int thread_pool_size,
325330

326331
void NodePlatform::RegisterIsolate(Isolate* isolate, uv_loop_t* loop) {
327332
Mutex::ScopedLock lock(per_isolate_mutex_);
328-
std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
329-
CHECK(!existing);
330-
per_isolate_[isolate] =
331-
std::make_shared<PerIsolatePlatformData>(isolate, loop);
333+
auto delegate = std::make_shared<PerIsolatePlatformData>(isolate, loop);
334+
IsolatePlatformDelegate* ptr = delegate.get();
335+
auto insertion = per_isolate_.emplace(
336+
isolate,
337+
std::make_pair(ptr, std::move(delegate)));
338+
CHECK(insertion.second);
339+
}
340+
341+
void NodePlatform::RegisterIsolate(Isolate* isolate,
342+
IsolatePlatformDelegate* delegate) {
343+
Mutex::ScopedLock lock(per_isolate_mutex_);
344+
auto insertion = per_isolate_.emplace(
345+
isolate,
346+
std::make_pair(delegate, std::shared_ptr<PerIsolatePlatformData>{}));
347+
CHECK(insertion.second);
332348
}
333349

334350
void NodePlatform::UnregisterIsolate(Isolate* isolate) {
335351
Mutex::ScopedLock lock(per_isolate_mutex_);
336-
std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate];
337-
CHECK(existing);
338-
existing->Shutdown();
339-
per_isolate_.erase(isolate);
352+
auto existing_it = per_isolate_.find(isolate);
353+
CHECK_NE(existing_it, per_isolate_.end());
354+
auto& existing = existing_it->second;
355+
if (existing.second) {
356+
existing.second->Shutdown();
357+
}
358+
per_isolate_.erase(existing_it);
340359
}
341360

342361
void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
343362
void (*cb)(void*), void* data) {
344363
Mutex::ScopedLock lock(per_isolate_mutex_);
345364
auto it = per_isolate_.find(isolate);
346365
if (it == per_isolate_.end()) {
347-
CHECK(it->second);
348366
cb(data);
349367
return;
350368
}
351-
it->second->AddShutdownCallback(cb, data);
369+
CHECK(it->second.second);
370+
it->second.second->AddShutdownCallback(cb, data);
352371
}
353372

354373
void NodePlatform::Shutdown() {
@@ -394,7 +413,7 @@ void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) {
394413
}
395414

396415
void NodePlatform::DrainTasks(Isolate* isolate) {
397-
std::shared_ptr<PerIsolatePlatformData> per_isolate = ForIsolate(isolate);
416+
std::shared_ptr<PerIsolatePlatformData> per_isolate = ForNodeIsolate(isolate);
398417

399418
do {
400419
// Worker tasks aren't associated with an Isolate.
@@ -452,23 +471,32 @@ void NodePlatform::CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
452471
}
453472

454473

474+
IsolatePlatformDelegate* NodePlatform::ForIsolate(Isolate* isolate) {
475+
Mutex::ScopedLock lock(per_isolate_mutex_);
476+
auto data = per_isolate_[isolate];
477+
CHECK_NOT_NULL(data.first);
478+
return data.first;
479+
}
480+
455481
std::shared_ptr<PerIsolatePlatformData>
456-
NodePlatform::ForIsolate(Isolate* isolate) {
482+
NodePlatform::ForNodeIsolate(Isolate* isolate) {
457483
Mutex::ScopedLock lock(per_isolate_mutex_);
458-
std::shared_ptr<PerIsolatePlatformData> data = per_isolate_[isolate];
459-
CHECK(data);
460-
return data;
484+
auto data = per_isolate_[isolate];
485+
CHECK(data.second);
486+
return data.second;
461487
}
462488

463489
bool NodePlatform::FlushForegroundTasks(Isolate* isolate) {
464-
return ForIsolate(isolate)->FlushForegroundTasksInternal();
490+
return ForNodeIsolate(isolate)->FlushForegroundTasksInternal();
465491
}
466492

467-
bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { return false; }
493+
bool NodePlatform::IdleTasksEnabled(Isolate* isolate) {
494+
return ForIsolate(isolate)->IdleTasksEnabled();
495+
}
468496

469497
std::shared_ptr<v8::TaskRunner>
470498
NodePlatform::GetForegroundTaskRunner(Isolate* isolate) {
471-
return ForIsolate(isolate);
499+
return ForIsolate(isolate)->GetForegroundTaskRunner();
472500
}
473501

474502
double NodePlatform::MonotonicallyIncreasingTime() {

‎src/node_platform.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,14 @@ struct DelayedTask {
5252

5353
// This acts as the foreground task runner for a given Isolate.
5454
class PerIsolatePlatformData :
55+
public IsolatePlatformDelegate,
5556
public v8::TaskRunner,
5657
public std::enable_shared_from_this<PerIsolatePlatformData> {
5758
public:
5859
PerIsolatePlatformData(v8::Isolate* isolate, uv_loop_t* loop);
5960
~PerIsolatePlatformData() override;
6061

62+
std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner() override;
6163
void PostTask(std::unique_ptr<v8::Task> task) override;
6264
void PostIdleTask(std::unique_ptr<v8::IdleTask> task) override;
6365
void PostDelayedTask(std::unique_ptr<v8::Task> task,
@@ -162,6 +164,9 @@ class NodePlatform : public MultiIsolatePlatform {
162164
bool FlushForegroundTasks(v8::Isolate* isolate) override;
163165

164166
void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
167+
void RegisterIsolate(v8::Isolate* isolate,
168+
IsolatePlatformDelegate* delegate) override;
169+
165170
void UnregisterIsolate(v8::Isolate* isolate) override;
166171
void AddIsolateFinishedCallback(v8::Isolate* isolate,
167172
void (*callback)(void*), void* data) override;
@@ -170,11 +175,13 @@ class NodePlatform : public MultiIsolatePlatform {
170175
v8::Isolate* isolate) override;
171176

172177
private:
173-
std::shared_ptr<PerIsolatePlatformData> ForIsolate(v8::Isolate* isolate);
178+
IsolatePlatformDelegate* ForIsolate(v8::Isolate* isolate);
179+
std::shared_ptr<PerIsolatePlatformData> ForNodeIsolate(v8::Isolate* isolate);
174180

175181
Mutex per_isolate_mutex_;
176-
std::unordered_map<v8::Isolate*,
177-
std::shared_ptr<PerIsolatePlatformData>> per_isolate_;
182+
using DelegatePair = std::pair<
183+
IsolatePlatformDelegate*, std::shared_ptr<PerIsolatePlatformData>>;
184+
std::unordered_map<v8::Isolate*, DelegatePair> per_isolate_;
178185

179186
node::tracing::TracingController* tracing_controller_;
180187
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;

‎test/cctest/node_test_fixture.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include "node_test_fixture.h"
22

3-
ArrayBufferUniquePtr NodeTestFixture::allocator{nullptr, nullptr};
4-
uv_loop_t NodeTestFixture::current_loop;
5-
NodePlatformUniquePtr NodeTestFixture::platform;
6-
TracingAgentUniquePtr NodeTestFixture::tracing_agent;
7-
bool NodeTestFixture::node_initialized = false;
3+
ArrayBufferUniquePtr NodeZeroIsolateTestFixture::allocator{nullptr, nullptr};
4+
uv_loop_t NodeZeroIsolateTestFixture::current_loop;
5+
NodePlatformUniquePtr NodeZeroIsolateTestFixture::platform;
6+
TracingAgentUniquePtr NodeZeroIsolateTestFixture::tracing_agent;
7+
bool NodeZeroIsolateTestFixture::node_initialized = false;

‎test/cctest/node_test_fixture.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,13 @@ using ArrayBufferUniquePtr = std::unique_ptr<node::ArrayBufferAllocator,
6060
using TracingAgentUniquePtr = std::unique_ptr<node::tracing::Agent>;
6161
using NodePlatformUniquePtr = std::unique_ptr<node::NodePlatform>;
6262

63-
class NodeTestFixture : public ::testing::Test {
63+
class NodeZeroIsolateTestFixture : public ::testing::Test {
6464
protected:
6565
static ArrayBufferUniquePtr allocator;
6666
static TracingAgentUniquePtr tracing_agent;
6767
static NodePlatformUniquePtr platform;
6868
static uv_loop_t current_loop;
6969
static bool node_initialized;
70-
v8::Isolate* isolate_;
7170

7271
static void SetUpTestCase() {
7372
if (!node_initialized) {
@@ -99,8 +98,18 @@ class NodeTestFixture : public ::testing::Test {
9998
void SetUp() override {
10099
allocator = ArrayBufferUniquePtr(node::CreateArrayBufferAllocator(),
101100
&node::FreeArrayBufferAllocator);
101+
}
102+
};
103+
104+
105+
class NodeTestFixture : public NodeZeroIsolateTestFixture {
106+
protected:
107+
v8::Isolate* isolate_;
108+
109+
void SetUp() override {
110+
NodeZeroIsolateTestFixture::SetUp();
102111
isolate_ = NewIsolate(allocator.get(), &current_loop);
103-
CHECK_NE(isolate_, nullptr);
112+
CHECK_NOT_NULL(isolate_);
104113
isolate_->Enter();
105114
}
106115

@@ -110,6 +119,7 @@ class NodeTestFixture : public ::testing::Test {
110119
isolate_->Dispose();
111120
platform->UnregisterIsolate(isolate_);
112121
isolate_ = nullptr;
122+
NodeZeroIsolateTestFixture::TearDown();
113123
}
114124
};
115125

‎test/cctest/test_platform.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,50 @@ TEST_F(PlatformTest, SkipNewTasksInFlushForegroundTasks) {
5757
EXPECT_EQ(3, run_count);
5858
EXPECT_FALSE(platform->FlushForegroundTasks(isolate_));
5959
}
60+
61+
// Tests the registration of an abstract `IsolatePlatformDelegate` instance as
62+
// opposed to the more common `uv_loop_s*` version of `RegisterIsolate`.
63+
TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
64+
// Allocate isolate
65+
v8::Isolate::CreateParams create_params;
66+
create_params.array_buffer_allocator = allocator.get();
67+
auto isolate = v8::Isolate::Allocate();
68+
CHECK_NOT_NULL(isolate);
69+
70+
// Register *first*, then initialize
71+
auto delegate = std::make_shared<node::PerIsolatePlatformData>(
72+
isolate,
73+
&current_loop);
74+
platform->RegisterIsolate(isolate, delegate.get());
75+
v8::Isolate::Initialize(isolate, create_params);
76+
77+
// Try creating Context + IsolateData + Environment
78+
{
79+
v8::Isolate::Scope isolate_scope(isolate);
80+
v8::HandleScope handle_scope(isolate);
81+
82+
auto context = node::NewContext(isolate);
83+
CHECK(!context.IsEmpty());
84+
v8::Context::Scope context_scope(context);
85+
86+
std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
87+
isolate_data{node::CreateIsolateData(isolate,
88+
&current_loop,
89+
platform.get()),
90+
node::FreeIsolateData};
91+
CHECK(isolate_data);
92+
93+
std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
94+
environment{node::CreateEnvironment(isolate_data.get(),
95+
context,
96+
0, nullptr,
97+
0, nullptr),
98+
node::FreeEnvironment};
99+
CHECK(environment);
100+
}
101+
102+
// Graceful shutdown
103+
delegate->Shutdown();
104+
isolate->Dispose();
105+
platform->UnregisterIsolate(isolate);
106+
}

0 commit comments

Comments
 (0)