worker: add mutex lock to MessagePort ctor

addaleax · addaleax · commit e70aa30ebddf · 2019-02-06T15:53:10.000+01:00
Automated tooling for race condition detection reports this as a possible problem. It’s unlikely that something bad would happen here (beyond maybe calling `TriggerAsync()` twice), but adding this should be okay. PR-URL: #25911 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -533,6 +533,11 @@ MessagePort* MessagePort::New(
   if (data) {
     port->Detach();
     port->data_ = std::move(data);
+
+    // This lock is here to avoid race conditions with the `owner_` read
+    // in AddToIncomingQueue(). (This would likely be unproblematic without it,
+    // but it's better to be safe than sorry.)
+    Mutex::ScopedLock lock(port->data_->mutex_);
     port->data_->owner_ = port;
     // If the existing MessagePortData object had pending messages, this is
     // the easiest way to run that queue.
