From 5fee18551362825ccbc93c18ecc1a012d0b8857d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 1 Aug 2018 21:50:45 +0200 Subject: [PATCH] worker: fix deadlock when calling terminate from exit handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just before we call the `'exit'` handlers of a Worker, we drain the public port’s message queue to ensure proper ordering. Previously, we held the Worker’s `mutex_` during the exit handler call, so calling `terminate()` on the worker could lead to a deadlock if called from one of those message handlers. This fixes flakiness in the `parallel/test-worker-dns-terminate` test. --- src/node_worker.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index d3bffba51f3954..06edb96804119b 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -287,22 +287,21 @@ void Worker::JoinThread() { } void Worker::OnThreadStopped() { - Mutex::ScopedLock lock(mutex_); - scheduled_on_thread_stopped_ = false; + { + Mutex::ScopedLock lock(mutex_); + scheduled_on_thread_stopped_ = false; - Debug(this, "Worker %llu thread stopped", thread_id_); + Debug(this, "Worker %llu thread stopped", thread_id_); - { - Mutex::ScopedLock stopped_lock(stopped_mutex_); - CHECK(stopped_); - } + { + Mutex::ScopedLock stopped_lock(stopped_mutex_); + CHECK(stopped_); + } - CHECK_EQ(child_port_, nullptr); - parent_port_ = nullptr; + CHECK_EQ(child_port_, nullptr); + parent_port_ = nullptr; + } - // It's okay to join the thread while holding the mutex because - // OnThreadStopped means it's no longer doing any work that might grab it - // and really just silently exiting. JoinThread(); { @@ -369,6 +368,7 @@ void Worker::StopThread(const FunctionCallbackInfo& args) { Worker* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); + Debug(w, "Worker %llu is getting stopped by parent", w->thread_id_); w->Exit(1); w->JoinThread(); }