Skip to content

Commit

Permalink
worker: release native Worker object earlier
Browse files Browse the repository at this point in the history
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: #26542
Fixes: #26535
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax authored and BridgeAR committed Mar 14, 2019
1 parent 9768ec4 commit 62801b9
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}

Expand Down Expand Up @@ -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_);
}

Expand Down Expand Up @@ -508,6 +507,10 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& 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);
Expand All @@ -517,6 +520,7 @@ void Worker::StartThread(const FunctionCallbackInfo<Value>& args) {
CHECK(w_->thread_stopper_.IsStopped());
w_->parent_port_ = nullptr;
w_->JoinThread();
delete w_;
});

uv_thread_options_t thread_options;
Expand Down Expand Up @@ -544,6 +548,7 @@ void Worker::StopThread(const FunctionCallbackInfo<Value>& 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<Value>& args) {
Expand Down

0 comments on commit 62801b9

Please sign in to comment.