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 id18555
push userbturner@mozilla.com
push dateFri, 04 Feb 2011 19:49:21 +0000
treeherdermozilla-central@9414d1eaeeeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssdwilsh, blocking
bugs631452
milestone2.0b12pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.
    */