From b4bc1a783623695342c22c34b1dc622117a2ad08 Mon Sep 17 00:00:00 2001
From: Eden Chuang <echuang@mozilla.com>
Date: Wed, 26 Feb 2025 08:02:02 +0000
Subject: [PATCH] Bug 1932468 - Making EventSourceImpl::mWorkerRef accessing be
 thread-safe. r=dom-worker-reviewers,asuth a=pascalc

EventSourceImpl::mWorkerRef is not accessed on the Worker thread only.
When EventSourceImpl::Dispatch is called, it could be on any thread, so the race could happen if Worker is shuttdown at the same time.
This patch uses DataMutex to protect EventSourceImpl::mWorkerRef.

Differential Revision: https://phabricator.services.mozilla.com/D239515
---
 dom/base/EventSource.cpp | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/dom/base/EventSource.cpp b/dom/base/EventSource.cpp
index 9e73f31fc62a..cc45f358ac6c 100644
--- a/dom/base/EventSource.cpp
+++ b/dom/base/EventSource.cpp
@@ -257,8 +257,9 @@ class EventSourceImpl final : public nsIObserver,
   nsString mLastFieldValue;
 
   // EventSourceImpl internal states.
-  // WorkerRef to keep the worker alive. (accessed on worker thread only)
-  RefPtr<ThreadSafeWorkerRef> mWorkerRef;
+  // WorkerRef to keep the worker alive.
+  DataMutex<RefPtr<ThreadSafeWorkerRef>> mWorkerRef;
+
   // Whether the window is frozen. May be set on main thread and read on target
   // thread.
   Atomic<bool> mFrozen;
@@ -372,6 +373,7 @@ EventSourceImpl::EventSourceImpl(EventSource* aEventSource,
                                  nsICookieJarSettings* aCookieJarSettings)
     : mReconnectionTime(0),
       mStatus(PARSE_STATE_OFF),
+      mWorkerRef(nullptr, "EventSourceImpl::mWorkerRef"),
       mFrozen(false),
       mGoingToDispatchAllMessages(false),
       mIsMainThread(NS_IsMainThread()),
@@ -1206,7 +1208,7 @@ void EventSourceImpl::ResetDecoder() {
 class CallRestartConnection final : public WorkerMainThreadRunnable {
  public:
   explicit CallRestartConnection(RefPtr<EventSourceImpl>&& aEventSourceImpl)
-      : WorkerMainThreadRunnable(aEventSourceImpl->mWorkerRef->Private(),
+      : WorkerMainThreadRunnable(GetCurrentThreadWorkerPrivate(),
                                  "EventSource :: RestartConnection"_ns),
         mESImpl(std::move(aEventSourceImpl)) {
     mWorkerPrivate->AssertIsOnWorkerThread();
@@ -1873,7 +1875,8 @@ class WorkerRunnableDispatcher final : public WorkerThreadRunnable {
 }  // namespace
 
 bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) {
-  MOZ_ASSERT(!mWorkerRef);
+  auto tsWorkerRef = mWorkerRef.Lock();
+  MOZ_ASSERT(!*tsWorkerRef);
   MOZ_ASSERT(aWorkerPrivate);
   aWorkerPrivate->AssertIsOnWorkerThread();
 
@@ -1889,14 +1892,15 @@ bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) {
     return false;
   }
 
-  mWorkerRef = new ThreadSafeWorkerRef(workerRef);
+  *tsWorkerRef = new ThreadSafeWorkerRef(workerRef);
   return true;
 }
 
 void EventSourceImpl::ReleaseWorkerRef() {
   MOZ_ASSERT(IsClosed());
   MOZ_ASSERT(IsCurrentThreadRunningWorker());
-  mWorkerRef = nullptr;
+  auto workerRef = mWorkerRef.Lock();
+  *workerRef = nullptr;
 }
 
 //-----------------------------------------------------------------------------
@@ -1925,10 +1929,15 @@ EventSourceImpl::Dispatch(already_AddRefed<nsIRunnable> aEvent,
 
   // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
   // runnable.
+  auto workerRef = mWorkerRef.Lock();
+  // Return NS_OK if the worker has already shutdown
+  if (!*workerRef) {
+    return NS_OK;
+  }
   RefPtr<WorkerRunnableDispatcher> event = new WorkerRunnableDispatcher(
-      this, mWorkerRef->Private(), event_ref.forget());
+      this, (*workerRef)->Private(), event_ref.forget());
 
-  if (!event->Dispatch(mWorkerRef->Private())) {
+  if (!event->Dispatch((*workerRef)->Private())) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;