diff --git a/src/env.cc b/src/env.cc index ba3e63ad8db7c6..271dbecf437c7a 100644 --- a/src/env.cc +++ b/src/env.cc @@ -81,8 +81,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))); @@ -134,12 +132,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)); } diff --git a/src/env.h b/src/env.h index b269982a0ba84d..5a368f93e6dc4d 100644 --- a/src/env.h +++ b/src/env.h @@ -399,7 +399,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 options(); diff --git a/src/node.h b/src/node.h index dffeb2b9cd7fc7..c52414b7e771a0 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 b96d5d3a1a8d70..1307af2879bee4 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -260,6 +260,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; @@ -268,21 +273,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(flush_tasks_), [](uv_handle_t* handle) { + std::unique_ptr callbacks( + static_cast(handle->data)); + for (const auto& callback : *callbacks) + callback.cb(callback.data); delete reinterpret_cast(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) { @@ -297,23 +300,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 existing = per_isolate_[isolate]; - if (existing) { - CHECK_EQ(loop, existing->event_loop()); - existing->ref(); - } else { - per_isolate_[isolate] = - std::make_shared(isolate, loop); - } + CHECK(!existing); + per_isolate_[isolate] = + std::make_shared(isolate, loop); } void NodePlatform::UnregisterIsolate(Isolate* isolate) { Mutex::ScopedLock lock(per_isolate_mutex_); std::shared_ptr 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 f99f0f9b9bd2b7..55a1e806180d34 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 task); static void RunForegroundTask(uv_timer_t* timer); - int ref_count_ = 1; + struct ShutdownCallback { + void (*cb)(void*); + void* data; + }; + typedef std::vector ShutdownCbList; + ShutdownCbList shutdown_callbacks_; + uv_loop_t* const loop_; uv_async_t* flush_tasks_ = nullptr; TaskQueue 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 GetForegroundTaskRunner( v8::Isolate* isolate) override; diff --git a/src/node_worker.cc b/src/node_worker.cc index 5ab4fad5d40114..f2d6a91ba171cf 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -186,16 +186,20 @@ class WorkerThreadData { w_->platform_->CancelPendingDelayedTasks(isolate); + bool platform_finished = false; + isolate_data_.reset(); + + w_->platform_->AddIsolateFinishedCallback(isolate, [](void* data) { + *static_cast(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_); }