tor-browser

commit a3ca81c0fa7b4bd8aa3f549bb9df65b454c7d4f8
parent 759209f8369c3778056cea6de7f2347549a9679f
Author: Jens Stutte <jstutte@mozilla.com>
Date:   Tue, 11 Nov 2025 14:08:53 +0000

Bug 1999509 - Ensure PostTimerEvent will not release anything important while holding the monitor's lock. r=xpcom-reviewers,nika

Short living event targets (like SharedThreadPool or TaskQueue or even dying threads)
must never be released while we hold TimerThread's monitor locked. PostTimerEvent needs
to keep a strong reference to the target before unlocking and must release that
reference before re-acquiring the lock. The same goes for nsTimerEvent.

Differential Revision: https://phabricator.services.mozilla.com/D257966

Diffstat:
M
xpcom/threads/TimerThread.cpp
 | 
24
++++++++++++------------

1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/xpcom/threads/TimerThread.cpp b/xpcom/threads/TimerThread.cpp
@@ -1230,25 +1230,25 @@ void TimerThread::PostTimerEvent(Entry& aPostMe) {
   // event, so we can avoid firing a timer that was re-initialized after being
   // canceled.
 
-  nsCOMPtr<nsIEventTarget> target = timer->mEventTarget;
-
   void* p = nsTimerEvent::operator new(sizeof(nsTimerEvent));
   if (!p) {
     return;
   }
-  RefPtr<nsTimerEvent> event = ::new (KnownNotNull, p)
-      nsTimerEvent(timer.forget(), aPostMe.mTimerSeq, mProfilerThreadId);
 
+  // We need to release mMonitor around the Dispatch because if the Dispatch
+  // or any Release of our objects interacts with the timer API we'll deadlock.
+
+  nsCOMPtr<nsIEventTarget> lockedTargetPtr = timer->mEventTarget;
+  RefPtr<nsTimerEvent> lockedEventPtr = ::new (KnownNotNull, p)
+      nsTimerEvent(timer.forget(), aPostMe.mTimerSeq, mProfilerThreadId);
   {
-    // We release mMonitor around the Dispatch because if the Dispatch interacts
-    // with the timer API we'll deadlock.
     MonitorAutoUnlock unlock(mMonitor);
-    if (NS_WARN_IF(NS_FAILED(target->Dispatch(event, NS_DISPATCH_NORMAL)))) {
-      // Dispatch may fail for an already shut down target. In that case
-      // we can't do much about it but drop the timer. We already removed
-      // its reference from our book-keeping, anyways.
-      RefPtr<nsTimerImpl> dropMe = event->ForgetTimer();
-    }
+    // Ensure references are released while we're unlocked.
+    nsCOMPtr<nsIEventTarget> target = lockedTargetPtr.forget();
+    RefPtr<nsTimerEvent> event = lockedEventPtr.forget();
+    // If we fail we have no way to report an error, but fallible dispatch
+    // will take care of releasing our event and timer.
+    target->Dispatch(event.forget(), NS_DISPATCH_FALLIBLE);
   }
 }