diff options
| -rw-r--r-- | src/env.cc | 8 | ||||
| -rw-r--r-- | src/env.h | 1 | ||||
| -rw-r--r-- | src/node.h | 14 | ||||
| -rw-r--r-- | src/node_platform.cc | 45 | ||||
| -rw-r--r-- | src/node_platform.h | 14 | ||||
| -rw-r--r-- | src/node_worker.cc | 14 |
6 files changed, 59 insertions, 37 deletions
diff --git a/src/env.cc b/src/env.cc index 5046aa0dd1..d791e41f92 100644 --- a/src/env.cc +++ b/src/env.cc @@ -84,8 +84,6 @@ IsolateData::IsolateData(Isolate* isolate, uses_node_allocator_(allocator_ == node_allocator_), platform_(platform) { CHECK_NOT_NULL(allocator_); - if (platform_ != nullptr) - platform_->RegisterIsolate(isolate_, event_loop); options_.reset( new PerIsolateOptions(*(per_process::cli_options->per_isolate))); @@ -137,12 +135,6 @@ IsolateData::IsolateData(Isolate* isolate, #undef V } -IsolateData::~IsolateData() { - if (platform_ != nullptr) - platform_->UnregisterIsolate(isolate_); -} - - void InitThreadLocalOnce() { CHECK_EQ(0, uv_key_create(&Environment::thread_local_env)); } @@ -401,7 +401,6 @@ class IsolateData { uv_loop_t* event_loop, MultiIsolatePlatform* platform = nullptr, ArrayBufferAllocator* node_allocator = nullptr); - ~IsolateData(); inline uv_loop_t* event_loop() const; inline MultiIsolatePlatform* platform() const; inline std::shared_ptr<PerIsolateOptions> options(); diff --git a/src/node.h b/src/node.h index 06b8bd0ff5..9ccab2e0f8 100644 --- a/src/node.h +++ b/src/node.h @@ -228,10 +228,22 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform { virtual void DrainTasks(v8::Isolate* isolate) = 0; virtual void CancelPendingDelayedTasks(v8::Isolate* isolate) = 0; - // These will be called by the `IsolateData` creation/destruction functions. + // This needs to be called between the calls to `Isolate::Allocate()` and + // `Isolate::Initialize()`, so that initialization can already start + // using the platform. + // When using `NewIsolate()`, this is taken care of by that function. + // This function may only be called once per `Isolate`. virtual void RegisterIsolate(v8::Isolate* isolate, struct uv_loop_s* loop) = 0; + // This needs to be called right before calling `Isolate::Dispose()`. + // This function may only be called once per `Isolate`. virtual void UnregisterIsolate(v8::Isolate* isolate) = 0; + // The platform should call the passed function once all state associated + // with the given isolate has been cleaned up. This can, but does not have to, + // happen asynchronously. + virtual void AddIsolateFinishedCallback(v8::Isolate* isolate, + void (*callback)(void*), + void* data) = 0; }; // Creates a new isolate with Node.js-specific settings. diff --git a/src/node_platform.cc b/src/node_platform.cc index ba5ca794c0..bb64502919 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -261,6 +261,11 @@ PerIsolatePlatformData::~PerIsolatePlatformData() { Shutdown(); } +void PerIsolatePlatformData::AddShutdownCallback(void (*callback)(void*), + void* data) { + shutdown_callbacks_.emplace_back(ShutdownCallback { callback, data }); +} + void PerIsolatePlatformData::Shutdown() { if (flush_tasks_ == nullptr) return; @@ -269,21 +274,19 @@ void PerIsolatePlatformData::Shutdown() { CHECK_NULL(foreground_tasks_.Pop()); CancelPendingDelayedTasks(); + ShutdownCbList* copy = new ShutdownCbList(std::move(shutdown_callbacks_)); + flush_tasks_->data = copy; uv_close(reinterpret_cast<uv_handle_t*>(flush_tasks_), [](uv_handle_t* handle) { + std::unique_ptr<ShutdownCbList> callbacks( + static_cast<ShutdownCbList*>(handle->data)); + for (const auto& callback : *callbacks) + callback.cb(callback.data); delete reinterpret_cast<uv_async_t*>(handle); }); flush_tasks_ = nullptr; } -void PerIsolatePlatformData::ref() { - ref_count_++; -} - -int PerIsolatePlatformData::unref() { - return --ref_count_; -} - NodePlatform::NodePlatform(int thread_pool_size, TracingController* tracing_controller) { if (tracing_controller) { @@ -298,23 +301,29 @@ 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]; - if (existing) { - CHECK_EQ(loop, existing->event_loop()); - existing->ref(); - } else { - per_isolate_[isolate] = - std::make_shared<PerIsolatePlatformData>(isolate, loop); - } + CHECK(!existing); + per_isolate_[isolate] = + std::make_shared<PerIsolatePlatformData>(isolate, loop); } void NodePlatform::UnregisterIsolate(Isolate* isolate) { Mutex::ScopedLock lock(per_isolate_mutex_); std::shared_ptr<PerIsolatePlatformData> existing = per_isolate_[isolate]; CHECK(existing); - if (existing->unref() == 0) { - existing->Shutdown(); - per_isolate_.erase(isolate); + existing->Shutdown(); + per_isolate_.erase(isolate); +} + +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); } void NodePlatform::Shutdown() { diff --git a/src/node_platform.h b/src/node_platform.h index f99f0f9b9b..55a1e80618 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -64,11 +64,9 @@ class PerIsolatePlatformData : double delay_in_seconds) override; bool IdleTasksEnabled() override { return false; } + void AddShutdownCallback(void (*callback)(void*), void* data); void Shutdown(); - void ref(); - int unref(); - // Returns true if work was dispatched or executed. New tasks that are // posted during flushing of the queue are postponed until the next // flushing. @@ -84,7 +82,13 @@ class PerIsolatePlatformData : static void RunForegroundTask(std::unique_ptr<v8::Task> task); static void RunForegroundTask(uv_timer_t* timer); - int ref_count_ = 1; + struct ShutdownCallback { + void (*cb)(void*); + void* data; + }; + typedef std::vector<ShutdownCallback> ShutdownCbList; + ShutdownCbList shutdown_callbacks_; + uv_loop_t* const loop_; uv_async_t* flush_tasks_ = nullptr; TaskQueue<v8::Task> foreground_tasks_; @@ -145,6 +149,8 @@ class NodePlatform : public MultiIsolatePlatform { void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override; void UnregisterIsolate(v8::Isolate* isolate) override; + void AddIsolateFinishedCallback(v8::Isolate* isolate, + void (*callback)(void*), void* data) override; std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner( v8::Isolate* isolate) override; diff --git a/src/node_worker.cc b/src/node_worker.cc index 6f290c3e2a..42d407e654 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -188,16 +188,20 @@ class WorkerThreadData { w_->platform_->CancelPendingDelayedTasks(isolate); + bool platform_finished = false; + isolate_data_.reset(); + + w_->platform_->AddIsolateFinishedCallback(isolate, [](void* data) { + *static_cast<bool*>(data) = true; + }, &platform_finished); w_->platform_->UnregisterIsolate(isolate); isolate->Dispose(); - // Need to run the loop twice more to close the platform's uv_async_t - // TODO(addaleax): It would be better for the platform itself to provide - // some kind of notification when it has fully cleaned up. - uv_run(&loop_, UV_RUN_ONCE); - uv_run(&loop_, UV_RUN_ONCE); + // Wait until the platform has cleaned up all relevant resources. + while (!platform_finished) + uv_run(&loop_, UV_RUN_ONCE); CheckedUvLoopClose(&loop_); } |