Skip to content

Commit

Permalink
src: add abstract IsolatePlatformDelegate
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
laverdet authored and addaleax committed Nov 20, 2019
1 parent c63af4f commit c712fb7
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 30 deletions.
12 changes: 12 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
66 changes: 47 additions & 19 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -267,7 +272,7 @@ void PerIsolatePlatformData::PostNonNestableDelayedTask(
}

PerIsolatePlatformData::~PerIsolatePlatformData() {
Shutdown();
CHECK(!flush_tasks_);
}

void PerIsolatePlatformData::AddShutdownCallback(void (*callback)(void*),
Expand Down Expand Up @@ -325,30 +330,44 @@ 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,
void (*cb)(void*), void* data) {
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() {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -452,23 +471,32 @@ void NodePlatform::CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
}


IsolatePlatformDelegate* NodePlatform::ForIsolate(Isolate* isolate) {
Mutex::ScopedLock lock(per_isolate_mutex_);
auto data = per_isolate_[isolate];
CHECK_NOT_NULL(data.first);
return data.first;
}

std::shared_ptr<PerIsolatePlatformData>
NodePlatform::ForIsolate(Isolate* isolate) {
NodePlatform::ForNodeIsolate(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(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() {
Expand Down
13 changes: 10 additions & 3 deletions src/node_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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_;
Expand Down
10 changes: 5 additions & 5 deletions test/cctest/node_test_fixture.cc
Original file line number Diff line number Diff line change
@@ -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;
16 changes: 13 additions & 3 deletions test/cctest/node_test_fixture.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}

Expand All @@ -110,6 +119,7 @@ class NodeTestFixture : public ::testing::Test {
isolate_->Dispose();
platform->UnregisterIsolate(isolate_);
isolate_ = nullptr;
NodeZeroIsolateTestFixture::TearDown();
}
};

Expand Down
47 changes: 47 additions & 0 deletions test/cctest/test_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit c712fb7

Please sign in to comment.