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>
This commit is contained in:
Anna Henningsen 2018-08-01 21:50:45 +02:00
parent fcb46a4626
commit e3bae65583
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9

View File

@ -287,22 +287,21 @@ void Worker::JoinThread() {
}
void Worker::OnThreadStopped() {
Mutex::ScopedLock lock(mutex_);
scheduled_on_thread_stopped_ = false;
Debug(this, "Worker %llu thread stopped", thread_id_);
{
Mutex::ScopedLock stopped_lock(stopped_mutex_);
CHECK(stopped_);
Mutex::ScopedLock lock(mutex_);
scheduled_on_thread_stopped_ = false;
Debug(this, "Worker %llu thread stopped", thread_id_);
{
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();
}