Bug 1695580 - In xpcom, cancel pending DelayedRunnable timers on shutdown. r=KrisWright

Because DelayedRunnables are fire-and-forget, there is no way for a targeted
EventTarget to clean them up on shutdown. Thus if a timer fires after
EventTarget shutdown it will fail to dispatch the timer event, and avoid
releasing the timer callback because it's not on the targeted thread. This
causes a leak as there is a ref-cycle between nsTimerImpl::mCallback and
DelayedRunnable::mTimer.

This patch adds nsIDelayedRunnableObserver for a target to observe which
DelayedRunnables are relying on their timer to run them. This allows the target
to schedule a shutdown task to cancel those timers and release the runnables on
the target thread.

Supported DelayedRunnable targets with this patch are TaskQueues,
eventqueue-based nsThreads and XPCOMThreadWrappers that wrap a supported
nsThread.

An assertion makes sure at runtime that future new uses of DelayedRunnable
target nsIDelayedRunnableObserver-supported event targets.

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

Author: Pehrsons

xpcom/build/XPCOMInit.cpp

@@ -621,6 +621,7 @@ nsresult ShutdownXPCOM(nsIServiceManager* aServMgr) {
 
     mozilla::AppShutdown::AdvanceShutdownPhase(
         mozilla::ShutdownPhase::XPCOMShutdownThreads);
+    nsThreadManager::get().CancelBackgroundDelayedRunnables();
     gXPCOMThreadsShutDown = true;
     NS_ProcessPendingEvents(thread);
 

xpcom/threads/AbstractThread.cpp

@@ -31,12 +31,14 @@ MOZ_THREAD_LOCAL(AbstractThread*) AbstractThread::sCurrentThreadTLS;
 
 class XPCOMThreadWrapper final : public AbstractThread,
                                  public nsIThreadObserver,
-                                 public nsIDirectTaskDispatcher {
+                                 public nsIDirectTaskDispatcher,
+                                 public nsIDelayedRunnableObserver {
  public:
   XPCOMThreadWrapper(nsIThreadInternal* aThread, bool aRequireTailDispatch,
                      bool aOnThread)
       : AbstractThread(aRequireTailDispatch),
         mThread(aThread),
+        mDelayedRunnableObserver(do_QueryInterface(mThread)),
         mDirectTaskDispatcher(do_QueryInterface(aThread)),
         mOnThread(aOnThread) {
     MOZ_DIAGNOSTIC_ASSERT(mThread && mDirectTaskDispatcher);
@@ -158,8 +160,22 @@ class XPCOMThreadWrapper final : public AbstractThread,
     return mDirectTaskDispatcher->HaveDirectTasks(aResult);
   }
 
+  //-----------------------------------------------------------------------------
+  // nsIDelayedRunnableObserver
+  //-----------------------------------------------------------------------------
+  void OnDelayedRunnableCreated(DelayedRunnable* aRunnable) override {
+    mDelayedRunnableObserver->OnDelayedRunnableCreated(aRunnable);
+  }
+  void OnDelayedRunnableScheduled(DelayedRunnable* aRunnable) override {
+    mDelayedRunnableObserver->OnDelayedRunnableScheduled(aRunnable);
+  }
+  void OnDelayedRunnableRan(DelayedRunnable* aRunnable) override {
+    mDelayedRunnableObserver->OnDelayedRunnableRan(aRunnable);
+  }
+
  private:
   const RefPtr<nsIThreadInternal> mThread;
+  const nsCOMPtr<nsIDelayedRunnableObserver> mDelayedRunnableObserver;
   const nsCOMPtr<nsIDirectTaskDispatcher> mDirectTaskDispatcher;
   Maybe<AutoTaskDispatcher> mTailDispatcher;
   const bool mOnThread;
@@ -215,8 +231,16 @@ class XPCOMThreadWrapper final : public AbstractThread,
     const RefPtr<nsIRunnable> mRunnable;
   };
 };
-NS_IMPL_ISUPPORTS_INHERITED(XPCOMThreadWrapper, AbstractThread,
-                            nsIThreadObserver, nsIDirectTaskDispatcher);
+
+NS_INTERFACE_MAP_BEGIN(XPCOMThreadWrapper)
+  NS_INTERFACE_MAP_ENTRY(nsIThreadObserver)
+  NS_INTERFACE_MAP_ENTRY(nsIDirectTaskDispatcher)
+  NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIDelayedRunnableObserver,
+                                     mDelayedRunnableObserver)
+NS_INTERFACE_MAP_END_INHERITING(AbstractThread)
+
+NS_IMPL_ADDREF_INHERITED(XPCOMThreadWrapper, AbstractThread)
+NS_IMPL_RELEASE_INHERITED(XPCOMThreadWrapper, AbstractThread)
 
 NS_IMPL_ISUPPORTS(AbstractThread, nsIEventTarget, nsISerialEventTarget)
 

xpcom/threads/DelayedRunnable.cpp

@@ -13,15 +13,25 @@ DelayedRunnable::DelayedRunnable(already_AddRefed<nsIEventTarget> aTarget,
                                  uint32_t aDelay)
     : mozilla::Runnable("DelayedRunnable"),
       mTarget(aTarget),
+      mObserver(do_QueryInterface(mTarget)),
       mWrappedRunnable(aRunnable),
       mDelayedFrom(TimeStamp::NowLoRes()),
-      mDelay(aDelay) {}
+      mDelay(aDelay) {
+  MOZ_DIAGNOSTIC_ASSERT(mObserver,
+                        "Target must implement nsIDelayedRunnableObserver");
+}
 
 nsresult DelayedRunnable::Init() {
+  mObserver->OnDelayedRunnableCreated(this);
   return NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, mDelay,
                                  nsITimer::TYPE_ONE_SHOT, mTarget);
 }
 
+void DelayedRunnable::CancelTimer() {
+  MOZ_ASSERT(mTarget->IsOnCurrentThread());
+  mTimer->Cancel();
+}
+
 NS_IMETHODIMP DelayedRunnable::Run() {
   MOZ_ASSERT(mTimer, "DelayedRunnable without Init?");
 
@@ -33,6 +43,9 @@ NS_IMETHODIMP DelayedRunnable::Run() {
   // Are we too early?
   if ((mozilla::TimeStamp::NowLoRes() - mDelayedFrom).ToMilliseconds() <
       mDelay) {
+    if (mObserver) {
+      mObserver->OnDelayedRunnableScheduled(this);
+    }
     return NS_OK;  // Let the nsITimer run us.
   }
 
@@ -44,6 +57,9 @@ NS_IMETHODIMP DelayedRunnable::Notify(nsITimer* aTimer) {
   // If we already ran, the timer should have been canceled.
   MOZ_ASSERT(mWrappedRunnable);
 
+  if (mObserver) {
+    mObserver->OnDelayedRunnableRan(this);
+  }
   return DoRun();
 }
 

xpcom/threads/DelayedRunnable.h

@@ -9,6 +9,7 @@
 
 #include "mozilla/TimeStamp.h"
 #include "nsCOMPtr.h"
+#include "nsIDelayedRunnableObserver.h"
 #include "nsIRunnable.h"
 #include "nsITimer.h"
 #include "nsThreadUtils.h"
@@ -26,11 +27,18 @@ class DelayedRunnable : public Runnable, public nsITimerCallback {
 
   nsresult Init();
 
+  /**
+   * Cancels the underlying timer. Called when the target is going away, so the
+   * runnable can be released safely on the target thread.
+   */
+  void CancelTimer();
+
  private:
   ~DelayedRunnable() = default;
   nsresult DoRun();
 
   const nsCOMPtr<nsIEventTarget> mTarget;
+  const nsCOMPtr<nsIDelayedRunnableObserver> mObserver;
   nsCOMPtr<nsIRunnable> mWrappedRunnable;
   nsCOMPtr<nsITimer> mTimer;
   const TimeStamp mDelayedFrom;

xpcom/threads/TaskQueue.cpp

@@ -6,6 +6,7 @@
 
 #include "mozilla/TaskQueue.h"
 
+#include "mozilla/DelayedRunnable.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
@@ -27,9 +28,11 @@ TaskQueue::TaskQueue(already_AddRefed<nsIEventTarget> aTarget,
 TaskQueue::~TaskQueue() {
   // No one is referencing this TaskQueue anymore, meaning no tasks can be
   // pending as all Runner hold a reference to this TaskQueue.
+  MOZ_ASSERT(mScheduledDelayedRunnables.IsEmpty());
 }
 
-NS_IMPL_ISUPPORTS_INHERITED(TaskQueue, AbstractThread, nsIDirectTaskDispatcher);
+NS_IMPL_ISUPPORTS_INHERITED(TaskQueue, AbstractThread, nsIDirectTaskDispatcher,
+                            nsIDelayedRunnableObserver);
 
 TaskDispatcher& TaskQueue::TailDispatcher() {
   MOZ_ASSERT(IsCurrentThreadIn());
@@ -104,13 +107,60 @@ void TaskQueue::AwaitShutdownAndIdle() {
   AwaitIdleLocked();
 }
 
+void TaskQueue::OnDelayedRunnableCreated(DelayedRunnable* aRunnable) {
+#ifdef DEBUG
+  MonitorAutoLock mon(mQueueMonitor);
+  MOZ_ASSERT(!mDelayedRunnablesCancelPromise);
+#endif
+}
+
+void TaskQueue::OnDelayedRunnableScheduled(DelayedRunnable* aRunnable) {
+  MOZ_ASSERT(IsOnCurrentThread());
+  mScheduledDelayedRunnables.AppendElement(aRunnable);
+}
+
+void TaskQueue::OnDelayedRunnableRan(DelayedRunnable* aRunnable) {
+  MOZ_ASSERT(IsOnCurrentThread());
+  MOZ_ALWAYS_TRUE(mScheduledDelayedRunnables.RemoveElement(aRunnable));
+}
+
+auto TaskQueue::CancelDelayedRunnables() -> RefPtr<CancelPromise> {
+  MonitorAutoLock mon(mQueueMonitor);
+  return CancelDelayedRunnablesLocked();
+}
+
+auto TaskQueue::CancelDelayedRunnablesLocked() -> RefPtr<CancelPromise> {
+  mQueueMonitor.AssertCurrentThreadOwns();
+  if (mDelayedRunnablesCancelPromise) {
+    return mDelayedRunnablesCancelPromise;
+  }
+  mDelayedRunnablesCancelPromise =
+      mDelayedRunnablesCancelHolder.Ensure(__func__);
+  nsCOMPtr<nsIRunnable> cancelRunnable =
+      NewRunnableMethod("TaskQueue::CancelDelayedRunnablesImpl", this,
+                        &TaskQueue::CancelDelayedRunnablesImpl);
+  MOZ_ALWAYS_SUCCEEDS(DispatchLocked(/* passed by ref */ cancelRunnable,
+                                     NS_DISPATCH_NORMAL, TailDispatch));
+  return mDelayedRunnablesCancelPromise;
+}
+
+void TaskQueue::CancelDelayedRunnablesImpl() {
+  MOZ_ASSERT(IsOnCurrentThread());
+  for (const auto& runnable : mScheduledDelayedRunnables) {
+    runnable->CancelTimer();
+  }
+  mScheduledDelayedRunnables.Clear();
+  mDelayedRunnablesCancelHolder.Resolve(true, __func__);
+}
+
 RefPtr<ShutdownPromise> TaskQueue::BeginShutdown() {
   // Dispatch any tasks for this queue waiting in the caller's tail dispatcher,
   // since this is the last opportunity to do so.
   if (AbstractThread* currentThread = AbstractThread::GetCurrent()) {
     currentThread->TailDispatchTasksFor(this);
   }
   MonitorAutoLock mon(mQueueMonitor);
+  Unused << CancelDelayedRunnablesLocked();
   mIsShutdown = true;
   RefPtr<ShutdownPromise> p = mShutdownPromise.Ensure(__func__);
   MaybeResolveShutdown();

xpcom/threads/TaskQueue.h

@@ -15,6 +15,7 @@
 #include "mozilla/MozPromise.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/TaskDispatcher.h"
+#include "nsIDelayedRunnableObserver.h"
 #include "nsIDirectTaskDispatcher.h"
 #include "nsThreadUtils.h"
 
@@ -47,7 +48,9 @@ typedef MozPromise<bool, bool, false> ShutdownPromise;
 // A TaskQueue does not require explicit shutdown, however it provides a
 // BeginShutdown() method that places TaskQueue in a shut down state and returns
 // a promise that gets resolved once all pending tasks have completed
-class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
+class TaskQueue : public AbstractThread,
+                  public nsIDirectTaskDispatcher,
+                  public nsIDelayedRunnableObserver {
   class EventTargetWrapper;
 
  public:
@@ -93,6 +96,18 @@ class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
   // So we can access nsIEventTarget::Dispatch(nsIRunnable*, uint32_t aFlags)
   using nsIEventTarget::Dispatch;
 
+  // nsIDelayedRunnableObserver
+  void OnDelayedRunnableCreated(DelayedRunnable* aRunnable) override;
+  void OnDelayedRunnableScheduled(DelayedRunnable* aRunnable) override;
+  void OnDelayedRunnableRan(DelayedRunnable* aRunnable) override;
+
+  using CancelPromise = MozPromise<bool, bool, false>;
+
+  // Dispatches a task to cancel any pending DelayedRunnables. Idempotent. Only
+  // dispatches the task on the first call. Creating DelayedRunnables after this
+  // is called will result in assertion failures.
+  RefPtr<CancelPromise> CancelDelayedRunnables();
+
   // Puts the queue in a shutdown state and returns immediately. The queue will
   // remain alive at least until all the events are drained, because the Runners
   // hold a strong reference to the task queue, and one of them is always held
@@ -126,6 +141,11 @@ class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
   nsresult DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable, uint32_t aFlags,
                           DispatchReason aReason = NormalDispatch);
 
+  RefPtr<CancelPromise> CancelDelayedRunnablesLocked();
+
+  // Cancels any scheduled DelayedRunnables on this TaskQueue.
+  void CancelDelayedRunnablesImpl();
+
   void MaybeResolveShutdown() {
     mQueueMonitor.AssertCurrentThreadOwns();
     if (mIsShutdown && !mIsRunning) {
@@ -136,7 +156,8 @@ class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
 
   nsCOMPtr<nsIEventTarget> mTarget;
 
-  // Monitor that protects the queue and mIsRunning;
+  // Monitor that protects the queue, mIsRunning and
+  // mDelayedRunnablesCancelPromise;
   Monitor mQueueMonitor;
 
   typedef struct TaskStruct {
@@ -147,6 +168,18 @@ class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
   // Queue of tasks to run.
   std::queue<TaskStruct> mTasks;
 
+  // DelayedRunnables (from DelayedDispatch) that are managed by their
+  // respective timers, but have not yet run. Accessed only on this
+  // TaskQueue.
+  nsTArray<RefPtr<DelayedRunnable>> mScheduledDelayedRunnables;
+
+  // Manages resolving mDelayedRunnablesCancelPromise.
+  MozPromiseHolder<CancelPromise> mDelayedRunnablesCancelHolder;
+
+  // Set once the task to cancel all pending DelayedRunnables has been
+  // dispatched. Guarded by mQueueMonitor.
+  RefPtr<CancelPromise> mDelayedRunnablesCancelPromise;
+
   // The thread currently running the task queue. We store a reference
   // to this so that IsCurrentThreadIn() can tell if the current thread
   // is the thread currently running in the task queue.

xpcom/threads/ThreadEventTarget.cpp

@@ -33,13 +33,18 @@ ThreadEventTarget::ThreadEventTarget(ThreadTargetSink* aSink,
   mThread = PR_GetCurrentThread();
 }
 
+ThreadEventTarget::~ThreadEventTarget() {
+  MOZ_ASSERT(mScheduledDelayedRunnables.IsEmpty());
+}
+
 void ThreadEventTarget::SetCurrentThread(PRThread* aThread) {
   mThread = aThread;
 }
 
 void ThreadEventTarget::ClearCurrentThread() { mThread = nullptr; }
 
-NS_IMPL_ISUPPORTS(ThreadEventTarget, nsIEventTarget, nsISerialEventTarget)
+NS_IMPL_ISUPPORTS(ThreadEventTarget, nsIEventTarget, nsISerialEventTarget,
+                  nsIDelayedRunnableObserver)
 
 NS_IMETHODIMP
 ThreadEventTarget::DispatchFromScript(nsIRunnable* aRunnable, uint32_t aFlags) {
@@ -136,3 +141,23 @@ ThreadEventTarget::IsOnCurrentThreadInfallible() {
   // we are called, we can never be on this thread.
   return false;
 }
+
+void ThreadEventTarget::OnDelayedRunnableCreated(DelayedRunnable* aRunnable) {}
+
+void ThreadEventTarget::OnDelayedRunnableScheduled(DelayedRunnable* aRunnable) {
+  MOZ_ASSERT(IsOnCurrentThread());
+  mScheduledDelayedRunnables.AppendElement(aRunnable);
+}
+
+void ThreadEventTarget::OnDelayedRunnableRan(DelayedRunnable* aRunnable) {
+  MOZ_ASSERT(IsOnCurrentThread());
+  MOZ_ALWAYS_TRUE(mScheduledDelayedRunnables.RemoveElement(aRunnable));
+}
+
+void ThreadEventTarget::NotifyShutdown() {
+  MOZ_ASSERT(IsOnCurrentThread());
+  for (const auto& runnable : mScheduledDelayedRunnables) {
+    runnable->CancelTimer();
+  }
+  mScheduledDelayedRunnables.Clear();
+}

xpcom/threads/ThreadEventTarget.h

@@ -10,19 +10,31 @@
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/SynchronizedEventQueue.h"  // for ThreadTargetSink
+#include "nsIDelayedRunnableObserver.h"
 #include "nsISerialEventTarget.h"
 
 namespace mozilla {
+class DelayedRunnable;
 
 // ThreadEventTarget handles the details of posting an event to a thread. It can
 // be used with any ThreadTargetSink implementation.
-class ThreadEventTarget final : public nsISerialEventTarget {
+class ThreadEventTarget final : public nsISerialEventTarget,
+                                public nsIDelayedRunnableObserver {
  public:
   ThreadEventTarget(ThreadTargetSink* aSink, bool aIsMainThread);
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIEVENTTARGET_FULL
 
+  // nsIDelayedRunnableObserver
+  void OnDelayedRunnableCreated(DelayedRunnable* aRunnable) override;
+  void OnDelayedRunnableScheduled(DelayedRunnable* aRunnable) override;
+  void OnDelayedRunnableRan(DelayedRunnable* aRunnable) override;
+
+  // Notification from, and on, the owner thread that it is shutting down.
+  // Cancels any scheduled DelayedRunnables.
+  void NotifyShutdown();
+
   // Disconnects the target so that it can no longer post events.
   void Disconnect(const MutexAutoLock& aProofOfLock) {
     mSink->Disconnect(aProofOfLock);
@@ -43,10 +55,14 @@ class ThreadEventTarget final : public nsISerialEventTarget {
   }
 
  private:
-  ~ThreadEventTarget() = default;
+  ~ThreadEventTarget();
 
   RefPtr<ThreadTargetSink> mSink;
   bool mIsMainThread;
+
+  // DelayedRunnables (from DelayedDispatch) that are managed by their
+  // respective timers, but have not yet run. Accessed only on this nsThread.
+  nsTArray<RefPtr<mozilla::DelayedRunnable>> mScheduledDelayedRunnables;
 };
 
 }  // namespace mozilla