Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break out platform delegate #30324

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,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 @@ -273,6 +279,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 {
platform->DrainTasks(isolate_);
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);
}