diff options
author | Santiago Gimeno <santiago.gimeno@gmail.com> | 2021-04-25 19:42:47 +0300 |
---|---|---|
committer | Juan José Arboleda <soyjuanarbol@gmail.com> | 2021-09-11 22:39:19 +0300 |
commit | 540f9d9c0f79a71a137d234eab668db6ebe28526 (patch) | |
tree | 906e5937577effcbedaa6065207e5eab7741f387 /src/node_worker.cc | |
parent | ec555b06d06f87e8467cf9b7f12978cdb88cb7bc (diff) |
worker: avoid potential deadlock on NearHeapLimit
It can happen that the `NearHeapLimit` callback is called while calling
the `oninit()` function on `MessagePort` construction causing a deadlock
when the `Worker::Exit()` method is called, as the `mutex_` was already
held on the `CreateEnvMessagePort()` method. To fix it, just use the
`mutex_` to protect the `child_port_data_` variable and avoid holding it
when creating the `MessagePort`.
Also, return early from `Worker::Run()` if the worker message port
could not be created.
Fixes: https://github.com/nodejs/node/issues/38208
PR-URL: https://github.com/nodejs/node/pull/38403
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Diffstat (limited to 'src/node_worker.cc')
-rw-r--r-- | src/node_worker.cc | 18 |
1 files changed, 14 insertions, 4 deletions
diff --git a/src/node_worker.cc b/src/node_worker.cc index 3195ddbaf01..dcc76ad88f0 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -328,7 +328,10 @@ void Worker::Run() { Debug(this, "Created Environment for worker with id %llu", thread_id_.id); if (is_stopped()) return; { - CreateEnvMessagePort(env_.get()); + if (!CreateEnvMessagePort(env_.get())) { + return; + } + Debug(this, "Created message port for worker %llu", thread_id_.id); if (LoadEnvironment(env_.get(), StartExecutionCallback{}).IsEmpty()) return; @@ -352,17 +355,24 @@ void Worker::Run() { Debug(this, "Worker %llu thread stops", thread_id_.id); } -void Worker::CreateEnvMessagePort(Environment* env) { +bool Worker::CreateEnvMessagePort(Environment* env) { HandleScope handle_scope(isolate_); - Mutex::ScopedLock lock(mutex_); + std::unique_ptr<MessagePortData> data; + { + Mutex::ScopedLock lock(mutex_); + data = std::move(child_port_data_); + } + // Set up the message channel for receiving messages in the child. MessagePort* child_port = MessagePort::New(env, env->context(), - std::move(child_port_data_)); + std::move(data)); // MessagePort::New() may return nullptr if execution is terminated // within it. if (child_port != nullptr) env->set_message_port(child_port->object(isolate_)); + + return child_port; } void Worker::JoinThread() { |