Bug 404870. Don't allow nsThread::PutEvent to succeed when the thread is no longer able to process the event. This stops us leaking the event and anything hanging off it. r+sr=bsmedberg

xpcom/threads/nsIEventTarget.idl

@@ -55,6 +55,9 @@ interface nsIEventTarget : nsISupports
    * 
    * @throws NS_ERROR_INVALID_ARG
    *   Indicates that event is null.
+   * @throws NS_ERROR_UNEXPECTED
+   *   Indicates that the thread is shutting down and has finished processing
+   * events, so this event would never run and has not been dispatched. 
    */
   void dispatch(in nsIRunnable event, in unsigned long flags);
 

xpcom/threads/nsThread.cpp

@@ -253,7 +253,25 @@ nsThread::ThreadFunc(void *arg)
   while (!self->ShuttingDown())
     NS_ProcessNextEvent(self);
 
-  NS_ProcessPendingEvents(self);
+  // Do NS_ProcessPendingEvents but with special handling to set
+  // mEventsAreDoomed atomically with the removal of the last event. The key
+  // invariant here is that we will never permit PutEvent to succeed if the
+  // event would be left in the queue after our final call to
+  // NS_ProcessPendingEvents.
+  while (PR_TRUE) {
+    {
+      nsAutoLock lock(self->mLock);
+      if (!self->mEvents->HasPendingEvent()) {
+        // No events in the queue, so we will stop now. Don't let any more
+        // events be added, since they won't be processed. It is critical
+        // that no PutEvent can occur between testing that the event queue is
+        // empty and setting mEventsAreDoomed!
+        self->mEventsAreDoomed = PR_TRUE;
+        break;
+      }
+    }
+    NS_ProcessPendingEvents(self);
+  }
 
   // Inform the threadmanager that this thread is going away
   nsThreadManager::get()->UnregisterCurrentThread(self);
@@ -275,6 +293,7 @@ nsThread::nsThread()
   , mRunningEvent(0)
   , mShutdownContext(nsnull)
   , mShutdownRequired(PR_FALSE)
+  , mEventsAreDoomed(PR_FALSE)
 {
 }
 
@@ -331,22 +350,24 @@ nsThread::InitCurrentThread()
   return NS_OK;
 }
 
-PRBool
+nsresult
 nsThread::PutEvent(nsIRunnable *event)
 {
-  PRBool rv;
   {
     nsAutoLock lock(mLock);
-    rv = mEvents->PutEvent(event);
+    if (mEventsAreDoomed) {
+      NS_WARNING("An event was posted to a thread that will never run it (rejected)");
+      return NS_ERROR_UNEXPECTED;
+    }
+    if (!mEvents->PutEvent(event))
+      return NS_ERROR_OUT_OF_MEMORY;
   }
-  if (!rv)
-    return PR_FALSE;
 
   nsCOMPtr<nsIThreadObserver> obs = GetObserver();
   if (obs)
     obs->OnDispatchedEvent(this);
 
-  return PR_TRUE;
+  return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
@@ -359,7 +380,6 @@ nsThread::Dispatch(nsIRunnable *event, PRUint32 flags)
 
   NS_ENSURE_ARG_POINTER(event);
 
-  PRBool dispatched;
   if (flags & DISPATCH_SYNC) {
     nsThread *thread = nsThreadManager::get()->GetCurrentThread();
     NS_ENSURE_STATE(thread);
@@ -372,19 +392,18 @@ nsThread::Dispatch(nsIRunnable *event, PRUint32 flags)
         new nsThreadSyncDispatch(thread, event);
     if (!wrapper)
       return NS_ERROR_OUT_OF_MEMORY;
-    dispatched = PutEvent(wrapper);
+    nsresult rv = PutEvent(wrapper);
+    // Don't wait for the event to finish if we didn't dispatch it...
+    if (NS_FAILED(rv))
+      return rv;
 
     while (wrapper->IsPending())
       NS_ProcessNextEvent(thread);
-  } else {
-    NS_ASSERTION(flags == NS_DISPATCH_NORMAL, "unexpected dispatch flags");
-    dispatched = PutEvent(event);
+    return rv;
   }
 
-  if (NS_UNLIKELY(!dispatched))
-    return NS_ERROR_OUT_OF_MEMORY;
-
-  return NS_OK;
+  NS_ASSERTION(flags == NS_DISPATCH_NORMAL, "unexpected dispatch flags");
+  return PutEvent(event);
 }
 
 NS_IMETHODIMP
@@ -434,6 +453,7 @@ nsThread::Shutdown()
   nsCOMPtr<nsIRunnable> event = new nsThreadShutdownEvent(this, &context);
   if (!event)
     return NS_ERROR_OUT_OF_MEMORY;
+  // XXXroc What if posting the event fails due to OOM?
   PutEvent(event);
 
   // We could still end up with other events being added after the shutdown

xpcom/threads/nsThread.h

@@ -95,7 +95,7 @@ private:
   PRBool GetEvent(PRBool mayWait, nsIRunnable **event) {
     return mEvents->GetEvent(mayWait, event);
   }
-  PRBool PutEvent(nsIRunnable *event);
+  nsresult PutEvent(nsIRunnable *event);
 
   // Wrapper for nsEventQueue that supports chaining.
   class nsChainedEventQueue {
@@ -113,6 +113,10 @@ private:
     }
 
     PRBool PutEvent(nsIRunnable *event);
+    
+    PRBool HasPendingEvent() {
+      return mQueue.HasPendingEvent();
+    }
 
     class nsChainedEventQueue *mNext;
   private:
@@ -120,11 +124,11 @@ private:
     nsEventQueue mQueue;
   };
 
-  // This lock protects access to mObserver and mEvents.  Both of those fields
-  // are only modified on the thread itself (never from another thread).  This
-  // means that we can avoid holding the lock while using mObserver and mEvents
-  // on the thread itself.  When calling PutEvent on mEvents, we have to hold
-  // the lock to synchronize with PopEventQueue.
+  // This lock protects access to mObserver, mEvents and mEventsAreDoomed.
+  // All of those fields are only modified on the thread itself (never from
+  // another thread).  This means that we can avoid holding the lock while
+  // using mObserver and mEvents on the thread itself.  When calling PutEvent
+  // on mEvents, we have to hold the lock to synchronize with PopEventQueue.
   PRLock *mLock;
 
   nsCOMPtr<nsIThreadObserver> mObserver;
@@ -140,6 +144,8 @@ private:
 
   PRPackedBool mShutdownRequired;
   PRPackedBool mShutdownPending;
+  // Set to true when events posted to this thread will never run.
+  PRPackedBool mEventsAreDoomed;
 };
 
 //-----------------------------------------------------------------------------