worker: fix deadlock when calling terminate from exit handler

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.

PR-URL: https://github.com/nodejs/node/pull/22073
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 12 insertions, 12 deletions

diff --git a/src/node_worker.cc b/src/node_worker.cc
index d3bffba51f..06edb96804 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<Value>& 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();
 }