From 62801b93202d389823286e429ad41fc24629b58a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 9 Mar 2019 12:37:06 +0100 Subject: [PATCH] worker: release native Worker object earlier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Destroy the `Worker` class earlier, because we don’t need access to it once the thread has stopped and all resources have been cleaned up. PR-URL: https://github.com/nodejs/node/pull/26542 Fixes: https://github.com/nodejs/node/issues/26535 Reviewed-By: Gireesh Punathil Reviewed-By: Joyee Cheung Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/node_worker.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index 982cf522a88e93..ca4d03f6d205e3 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -136,6 +136,9 @@ Worker::Worker(Environment* env, env->inspector_agent()->GetParentHandle(thread_id_, url); #endif + // Mark this Worker object as weak until we actually start the thread. + MakeWeak(); + Debug(this, "Preparation for worker %llu finished", thread_id_); } @@ -412,14 +415,10 @@ void Worker::OnThreadStopped() { Worker::~Worker() { Mutex::ScopedLock lock(mutex_); - JoinThread(); CHECK(thread_stopper_.IsStopped()); CHECK(thread_joined_); - // This has most likely already happened within the worker thread -- this - // is just in case Worker creation failed early. - Debug(this, "Worker %llu destroyed", thread_id_); } @@ -508,6 +507,10 @@ void Worker::StartThread(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); Mutex::ScopedLock lock(w->mutex_); + // The object now owns the created thread and should not be garbage collected + // until that finishes. + w->ClearWeak(); + w->env()->add_sub_worker_context(w); w->thread_joined_ = false; w->thread_stopper_.SetStopped(false); @@ -517,6 +520,7 @@ void Worker::StartThread(const FunctionCallbackInfo& args) { CHECK(w_->thread_stopper_.IsStopped()); w_->parent_port_ = nullptr; w_->JoinThread(); + delete w_; }); uv_thread_options_t thread_options; @@ -544,6 +548,7 @@ void Worker::StopThread(const FunctionCallbackInfo& args) { Debug(w, "Worker %llu is getting stopped by parent", w->thread_id_); w->Exit(1); w->JoinThread(); + delete w; } void Worker::Ref(const FunctionCallbackInfo& args) {