diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-08-01 21:50:45 +0200 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2018-08-09 02:11:55 +0200 |
commit | e3bae655839ea7f0019f7e6bd33fe941a5f6be62 (patch) | |
tree | 96184a8376eec20f3f52faa16b35a64d285ea3f6 | |
parent | fcb46a462635361daa922e9b38a1b7dbda461c8a (diff) | |
download | android-node-v8-e3bae655839ea7f0019f7e6bd33fe941a5f6be62.tar.gz android-node-v8-e3bae655839ea7f0019f7e6bd33fe941a5f6be62.tar.bz2 android-node-v8-e3bae655839ea7f0019f7e6bd33fe941a5f6be62.zip |
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>
-rw-r--r-- | src/node_worker.cc | 24 |
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(); } |