diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-05-16 13:03:32 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2020-05-20 00:56:57 +0300 |
commit | 0e92ae64f01140bc527b5a02d7f154dca882ef09 (patch) | |
tree | d90a99a393e8814fccd5dee08acd8631474b78c3 /src/node_messaging.cc | |
parent | 61189d3981ef49885f8d98eef2ac0207b6e59f40 (diff) |
worker: fix race condition in node_messaging.cc
`AddToIncomingQueue()` relies on `owner_` only being modified with
`mutex_` being locked, but in these two places, that didn’t happen.
Modify them to use `Detach()` instead, which has the same effect
as setting `owner_ = nullptr` here, but does it with proper locking.
This race condition probably only shows up in practice when Node.js
is compiled in debug mode, because the compiler eliminates the
duplicate load in `AddToIncomingQueue()` when compiling with
optimizations enabled.
PR-URL: https://github.com/nodejs/node/pull/33429
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'src/node_messaging.cc')
-rw-r--r-- | src/node_messaging.cc | 8 |
1 files changed, 3 insertions, 5 deletions
diff --git a/src/node_messaging.cc b/src/node_messaging.cc index d02a10c9740..6081a523cd4 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -464,8 +464,7 @@ void MessagePortData::Disentangle() { } MessagePort::~MessagePort() { - if (data_) - data_->owner_ = nullptr; + if (data_) Detach(); } MessagePort::MessagePort(Environment* env, @@ -662,10 +661,9 @@ void MessagePort::OnMessage() { void MessagePort::OnClose() { Debug(this, "MessagePort::OnClose()"); if (data_) { - data_->owner_ = nullptr; - data_->Disentangle(); + // Detach() returns move(data_). + Detach()->Disentangle(); } - data_.reset(); } std::unique_ptr<MessagePortData> MessagePort::Detach() { |