# HG changeset patch
# User Andrew Osmond <aosmond@mozilla.com>
# Date 1522162621 14400
#      Tue Mar 27 10:57:01 2018 -0400
# Node ID 1c5d4e9652092cb2cbec3c42656486a09b84ec9f
# Parent  78ff660c741474a4e439377a3e73f244d575e966
Bug 1444537 - Part 2. Shutting down the decode pool should make animated decoders bail early. r=tnikkel

When we shutdown the decode pool threads, it does not do a simple join
with the main thread. It will actually process the main thread event
loop, which can cause a bad series of events. The refresh tick could
still be running and advancing our animated images, causing the animated
decoders to continue running, which in turn prevents the decoder threads
from finishing shutting down, and the main thread from joining them.

Now we check on each frame whether or not the decoder should just stop
decoding more frames because the decode pool has started shutdown. If it
has, it will stop immediately.

diff --git a/image/AnimationSurfaceProvider.cpp b/image/AnimationSurfaceProvider.cpp
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -289,17 +289,20 @@ AnimationSurfaceProvider::CheckForNewFra
       justGotFirstFrame = true;
     }
   }
 
   if (justGotFirstFrame) {
     AnnounceSurfaceAvailable();
   }
 
-  return continueDecoding;
+  // If we are shutting down, we want to ensure we release the thread as soon
+  // as possible. The animation may advance even during shutdown, which keeps
+  // us decoding, and thus blocking the decode pool during teardown.
+  return continueDecoding && !DecodePool::Singleton()->IsShuttingDown();
 }
 
 bool
 AnimationSurfaceProvider::CheckForNewFrameAtTerminalState()
 {
   mDecodingMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mDecoder);
 
@@ -339,17 +342,20 @@ AnimationSurfaceProvider::CheckForNewFra
       justGotFirstFrame = true;
     }
   }
 
   if (justGotFirstFrame) {
     AnnounceSurfaceAvailable();
   }
 
-  return continueDecoding;
+  // If we are shutting down, we want to ensure we release the thread as soon
+  // as possible. The animation may advance even during shutdown, which keeps
+  // us decoding, and thus blocking the decode pool during teardown.
+  return continueDecoding && !DecodePool::Singleton()->IsShuttingDown();
 }
 
 void
 AnimationSurfaceProvider::AnnounceSurfaceAvailable()
 {
   mFramesMutex.AssertNotCurrentThreadOwns();
   MOZ_ASSERT(mImage);
 
diff --git a/image/DecodePool.cpp b/image/DecodePool.cpp
--- a/image/DecodePool.cpp
+++ b/image/DecodePool.cpp
@@ -112,16 +112,22 @@ public:
       mMonitor.NotifyAll();
     }
 
     for (uint32_t i = 0 ; i < threads.Length() ; ++i) {
       threads[i]->Shutdown();
     }
   }
 
+  bool IsShuttingDown() const
+  {
+    MonitorAutoLock lock(mMonitor);
+    return mShuttingDown;
+  }
+
   /// Pushes a new decode work item.
   void PushWork(IDecodingTask* aTask)
   {
     MOZ_ASSERT(aTask);
     RefPtr<IDecodingTask> task(aTask);
 
     MonitorAutoLock lock(mMonitor);
 
@@ -237,17 +243,17 @@ private:
     Work work;
     work.mType = Work::Type::SHUTDOWN;
     return work;
   }
 
   nsThreadPoolNaming mThreadNaming;
 
   // mMonitor guards everything below.
-  Monitor mMonitor;
+  mutable Monitor mMonitor;
   nsTArray<RefPtr<IDecodingTask>> mHighPriorityQueue;
   nsTArray<RefPtr<IDecodingTask>> mLowPriorityQueue;
   nsTArray<nsCOMPtr<nsIThread>> mThreads;
   PRIntervalTime mIdleTimeout;
   uint8_t mMaxIdleThreads;   // Maximum number of workers when idle.
   uint8_t mAvailableThreads; // How many new threads can be created.
   uint8_t mIdleThreads; // How many created threads are waiting.
   bool mShuttingDown;
@@ -427,16 +433,22 @@ DecodePool::Observe(nsISupports*, const 
 
   if (ioThread) {
     ioThread->Shutdown();
   }
 
   return NS_OK;
 }
 
+bool
+DecodePool::IsShuttingDown() const
+{
+  return mImpl->IsShuttingDown();
+}
+
 void
 DecodePool::AsyncRun(IDecodingTask* aTask)
 {
   MOZ_ASSERT(aTask);
   mImpl->PushWork(aTask);
 }
 
 bool
diff --git a/image/DecodePool.h b/image/DecodePool.h
--- a/image/DecodePool.h
+++ b/image/DecodePool.h
@@ -50,16 +50,20 @@ public:
 
   /// Returns the singleton instance.
   static DecodePool* Singleton();
 
   /// @return the number of processor cores we have available. This is not the
   /// same as the number of decoding threads we're actually using.
   static uint32_t NumberOfCores();
 
+  /// True if the DecodePool is being shutdown. This may only be called by
+  /// threads from the pool to check if they should keep working or not.
+  bool IsShuttingDown() const;
+
   /// Ask the DecodePool to run @aTask asynchronously and return immediately.
   void AsyncRun(IDecodingTask* aTask);
 
   /**
    * Run @aTask synchronously if the task would prefer it. It's up to the task
    * itself to make this decision; @see IDecodingTask::ShouldPreferSyncRun(). If
    * @aTask doesn't prefer it, just run @aTask asynchronously and return
    * immediately.