Bug 631452 - 'LazyIdleThread can race with newly dispatched events when shutting down'. r=sdwilsh, a=blocking.
authorBen Turner <bent.mozilla@gmail.com>
Fri, 04 Feb 2011 11:48:59 -0800
changeset 61948 9414d1eaeeeb0342fc6e44e6c4efcdb10f213803
parent 61947 313e35cc080e4aa039fef2dea78322ee5cf78bc4
child 61949 cff79fb2f133b50e5349b6edf27e746407b7038f
push idunknown
push userunknown
push dateunknown
reviewerssdwilsh, blocking
bugs631452
milestone2.0b12pre
Bug 631452 - 'LazyIdleThread can race with newly dispatched events when shutting down'. r=sdwilsh, a=blocking.
dom/indexedDB/LazyIdleThread.cpp
dom/indexedDB/LazyIdleThread.h
--- a/dom/indexedDB/LazyIdleThread.cpp
+++ b/dom/indexedDB/LazyIdleThread.cpp
@@ -64,16 +64,17 @@ USING_INDEXEDDB_NAMESPACE
 using mozilla::MutexAutoLock;
 
 LazyIdleThread::LazyIdleThread(PRUint32 aIdleTimeoutMS,
                                ShutdownMethod aShutdownMethod,
                                nsIObserver* aIdleObserver)
 : mMutex("LazyIdleThread::mMutex"),
   mOwningThread(NS_GetCurrentThread()),
   mIdleObserver(aIdleObserver),
+  mQueuedRunnables(nsnull),
   mIdleTimeoutMS(aIdleTimeoutMS),
   mPendingEventCount(0),
   mIdleNotificationCount(0),
   mShutdownMethod(aShutdownMethod),
   mShutdown(PR_FALSE),
   mThreadIsShuttingDown(PR_FALSE),
   mIdleTimeoutEnabled(PR_TRUE)
 {
@@ -251,16 +252,21 @@ LazyIdleThread::ScheduleTimer()
   }
 }
 
 nsresult
 LazyIdleThread::ShutdownThread()
 {
   ASSERT_OWNING_THREAD();
 
+  // Before calling Shutdown() on the real thread we need to put a queue in
+  // place in case a runnable is posted to the thread while it's in the
+  // process of shutting down. This will be our queue.
+  nsAutoTArray<nsCOMPtr<nsIRunnable>, 10> queuedRunnables;
+
   nsresult rv;
 
   if (mThread) {
     if (mShutdownMethod == AutomaticShutdown && NS_IsMainThread()) {
       nsCOMPtr<nsIObserverService> obs =
         do_GetService(NS_OBSERVERSERVICE_CONTRACTID);
       NS_WARN_IF_FALSE(obs, "Failed to get observer service!");
 
@@ -286,18 +292,25 @@ LazyIdleThread::ShutdownThread()
       NS_NewRunnableMethod(this, &LazyIdleThread::CleanupThread);
     NS_ENSURE_TRUE(runnable, NS_ERROR_FAILURE);
 
     PreDispatch();
 
     rv = mThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = mThread->Shutdown();
-    NS_ENSURE_SUCCESS(rv, rv);
+    // Put the temporary queue in place before calling Shutdown().
+    mQueuedRunnables = &queuedRunnables;
+
+    if (NS_FAILED(mThread->Shutdown())) {
+      NS_ERROR("Failed to shutdown the thread!");
+    }
+
+    // Now unset the queue.
+    mQueuedRunnables = nsnull;
 
     mThread = nsnull;
 
     {
       MutexAutoLock lock(mMutex);
 
       NS_ASSERTION(!mPendingEventCount, "Huh?!");
       NS_ASSERTION(!mIdleNotificationCount, "Huh?!");
@@ -308,16 +321,36 @@ LazyIdleThread::ShutdownThread()
 
   if (mIdleTimer) {
     rv = mIdleTimer->Cancel();
     NS_ENSURE_SUCCESS(rv, rv);
 
     mIdleTimer = nsnull;
   }
 
+  // If our temporary queue has any runnables then we need to dispatch them.
+  if (queuedRunnables.Length()) {
+    // If the thread manager has gone away then these runnables will never run.
+    if (mShutdown) {
+      NS_ERROR("Runnables dispatched to LazyIdleThread will never run!");
+      return NS_OK;
+    }
+
+    // Re-dispatch the queued runnables.
+    for (PRUint32 index = 0; index < queuedRunnables.Length(); index++) {
+      nsCOMPtr<nsIRunnable> runnable;
+      runnable.swap(queuedRunnables[index]);
+      NS_ASSERTION(runnable, "Null runnable?!");
+
+      if (NS_FAILED(Dispatch(runnable, NS_DISPATCH_NORMAL))) {
+        NS_ERROR("Failed to re-dispatch queued runnable!");
+      }
+    }
+  }
+
   return NS_OK;
 }
 
 void
 LazyIdleThread::SelfDestruct()
 {
   NS_ASSERTION(mRefCnt == 1, "Bad refcount!");
   delete this;
@@ -359,16 +392,26 @@ NS_IMPL_THREADSAFE_QUERY_INTERFACE5(Lazy
                                                     nsIObserver)
 
 NS_IMETHODIMP
 LazyIdleThread::Dispatch(nsIRunnable* aEvent,
                          PRUint32 aFlags)
 {
   ASSERT_OWNING_THREAD();
 
+  // LazyIdleThread can't always support synchronous dispatch currently.
+  NS_ENSURE_TRUE(aFlags == NS_DISPATCH_NORMAL, NS_ERROR_NOT_IMPLEMENTED);
+
+  // If our thread is shutting down then we can't actually dispatch right now.
+  // Queue this runnable for later.
+  if (UseRunnableQueue()) {
+    mQueuedRunnables->AppendElement(aEvent);
+    return NS_OK;
+  }
+
   nsresult rv = EnsureThread();
   NS_ENSURE_SUCCESS(rv, rv);
 
   PreDispatch();
 
   return mThread->Dispatch(aEvent, aFlags);
 }
 
@@ -394,20 +437,21 @@ LazyIdleThread::GetPRThread(PRThread** a
   return NS_ERROR_NOT_AVAILABLE;
 }
 
 NS_IMETHODIMP
 LazyIdleThread::Shutdown()
 {
   ASSERT_OWNING_THREAD();
 
+  mShutdown = PR_TRUE;
+
   nsresult rv = ShutdownThread();
   NS_ASSERTION(!mThread, "Should have destroyed this by now!");
 
-  mShutdown = PR_TRUE;
   mIdleObserver = nsnull;
 
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/dom/indexedDB/LazyIdleThread.h
+++ b/dom/indexedDB/LazyIdleThread.h
@@ -146,16 +146,24 @@ private:
 
   /**
    * Deletes this object. Used to delay calling mThread->Shutdown() during the
    * final release (during a GC, for instance).
    */
   void SelfDestruct();
 
   /**
+   * Returns true if events should be queued rather than immediately dispatched
+   * to mThread. Currently only happens when the thread is shutting down.
+   */
+  PRBool UseRunnableQueue() {
+    return !!mQueuedRunnables;
+  }
+
+  /**
    * Protects data that is accessed on both threads.
    */
   mozilla::Mutex mMutex;
 
   /**
    * Touched on both threads but set before mThread is created. Used to direct
    * timer events to the owning thread.
    */
@@ -175,16 +183,22 @@ private:
 
   /**
    * Idle observer. Called when the thread is about to be shut down. Released
    * only when Shutdown() is called.
    */
   nsIObserver* mIdleObserver;
 
   /**
+   * Temporary storage for events that happen to be dispatched while we're in
+   * the process of shutting down our real thread.
+   */
+  nsTArray<nsCOMPtr<nsIRunnable> >* mQueuedRunnables;
+
+  /**
    * The number of milliseconds a thread should be idle before dying.
    */
   const PRUint32 mIdleTimeoutMS;
 
   /**
    * The number of events that are pending on mThread. A nonzero value means
    * that the thread cannot be cleaned up.
    */