Bug 857367 - Make it safe to call RasterImage::DecodePool::RequestDecode off the main thread. r=tn
authorSeth Fowler <seth@mozilla.com>
Thu, 04 Apr 2013 18:01:41 -0700
changeset 137382 f15def8141847ebd3d4e037aeef2ca7044308e35
parent 137381 88773e6debe38f356c6c037581da9c015e993b99
child 137383 14d9c6c56dc6d35faae274d9013bdeb7f1dd92f8
push id2452
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 16:59:38 +0000
treeherdermozilla-beta@d4b152d29d8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs857367
milestone22.0a2
Bug 857367 - Make it safe to call RasterImage::DecodePool::RequestDecode off the main thread. r=tn
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -3606,16 +3606,18 @@ RasterImage::DecodePool::Singleton()
     sSingleton = new DecodePool();
     ClearOnShutdown(&sSingleton);
   }
 
   return sSingleton;
 }
 
 RasterImage::DecodePool::DecodePool()
+  : mThreadPoolMutex("Thread Pool")
+  , mShuttingDown(false)
 {
   if (gMultithreadedDecoding) {
     mThreadPool = do_CreateInstance(NS_THREADPOOL_CONTRACTID);
     if (mThreadPool) {
       mThreadPool->SetName(NS_LITERAL_CSTRING("ImageDecoder"));
       if (gDecodingThreadLimit <= 0) {
         mThreadPool->SetThreadLimit(std::max(PR_GetNumberOfProcessors() - 1, 1));
       } else {
@@ -3632,33 +3634,40 @@ RasterImage::DecodePool::DecodePool()
 
 RasterImage::DecodePool::~DecodePool()
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must shut down DecodePool on main thread!");
 }
 
 NS_IMETHODIMP
 RasterImage::DecodePool::Observe(nsISupports *subject, const char *topic,
-                                   const PRUnichar *data)
+                                 const PRUnichar *data)
 {
   NS_ASSERTION(strcmp(topic, "xpcom-shutdown-threads") == 0, "oops");
 
-  if (mThreadPool) {
-    mThreadPool->Shutdown();
+  nsCOMPtr<nsIThreadPool> threadPool;
+
+  {
+    MutexAutoLock threadPoolLock(mThreadPoolMutex);
+    threadPool = mThreadPool;
     mThreadPool = nullptr;
+    mShuttingDown = true;
+  }
+
+  if (threadPool) {
+    threadPool->Shutdown();
   }
 
   return NS_OK;
 }
 
 void
 RasterImage::DecodePool::RequestDecode(RasterImage* aImg)
 {
   MOZ_ASSERT(aImg->mDecoder);
-  MOZ_ASSERT(NS_IsMainThread());
   aImg->mDecodingMutex.AssertCurrentThreadOwns();
 
   // If we're currently waiting on a new frame for this image, we can't do any
   // decoding.
   if (!aImg->mDecoder->NeedsNewFrame()) {
     // No matter whether this is currently being decoded, we need to update the
     // number of bytes we want it to decode.
     aImg->mDecodeRequest->mBytesToDecode = aImg->mSourceData.Length() - aImg->mBytesDecoded;
@@ -3667,17 +3676,20 @@ RasterImage::DecodePool::RequestDecode(R
         aImg->mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_ACTIVE) {
       // The image is already in our list of images to decode, or currently being
       // decoded, so we don't have to do anything else.
       return;
     }
 
     aImg->mDecodeRequest->mRequestStatus = DecodeRequest::REQUEST_PENDING;
     nsRefPtr<DecodeJob> job = new DecodeJob(aImg->mDecodeRequest, aImg);
-    if (!gMultithreadedDecoding || !mThreadPool) {
+    MutexAutoLock threadPoolLock(mThreadPoolMutex);
+    if (mShuttingDown) {
+      // Just drop the job on the floor; we won't need it.
+    } else if (!gMultithreadedDecoding || !mThreadPool) {
       NS_DispatchToMainThread(job);
     } else {
       mThreadPool->Dispatch(job, nsIEventTarget::DISPATCH_NORMAL);
     }
   }
 }
 
 void
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -522,17 +522,22 @@ private:
 
     private:
       nsRefPtr<DecodeRequest> mRequest;
       nsRefPtr<RasterImage> mImage;
     };
 
   private: /* members */
 
+    // mThreadPoolMutex protects both mThreadPool and mShuttingDown. For all
+    // RasterImages R, R::mDecodingMutex must be acquired before
+    // mThreadPoolMutex if both are acquired; the other order may cause deadlock.
+    mozilla::Mutex          mThreadPoolMutex;
     nsCOMPtr<nsIThreadPool> mThreadPool;
+    bool                    mShuttingDown;
   };
 
   class DecodeDoneWorker : public nsRunnable
   {
   public:
     /**
      * Called by the DecodePool with an image when it's done some significant
      * portion of decoding that needs to be notified about.