Bug 1293577 - Protect the image decoding task path with a mutex to avoid race conditions. r=seth
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 10 Aug 2016 07:35:07 -0400
changeset 352386 64863b1fcf6470de45a6f3979dd3b1709e4b10e8
parent 352385 7db46aeeaf03b71414b656d605edc3ad98dae176
child 352387 9bd1de57490e862353ee3afad9a647fcfa8ff909
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1293577
milestone51.0a1
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 1293577 - Protect the image decoding task path with a mutex to avoid race conditions. r=seth
image/DecodedSurfaceProvider.cpp
image/DecodedSurfaceProvider.h
image/IDecodingTask.cpp
image/IDecodingTask.h
--- a/image/DecodedSurfaceProvider.cpp
+++ b/image/DecodedSurfaceProvider.cpp
@@ -15,16 +15,17 @@ using namespace mozilla::gfx;
 namespace mozilla {
 namespace image {
 
 DecodedSurfaceProvider::DecodedSurfaceProvider(NotNull<RasterImage*> aImage,
                                                NotNull<Decoder*> aDecoder,
                                                const SurfaceKey& aSurfaceKey)
   : ISurfaceProvider(AvailabilityState::StartAsPlaceholder())
   , mImage(aImage.get())
+  , mMutex("mozilla::image::DecodedSurfaceProvider")
   , mDecoder(aDecoder.get())
   , mSurfaceKey(aSurfaceKey)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
   MOZ_ASSERT(mDecoder->IsFirstFrameDecode(),
              "Use AnimationDecodingTask for animation decodes");
 }
@@ -119,16 +120,18 @@ DecodedSurfaceProvider::LogicalSizeInByt
   // Single frame images are always 32bpp.
   IntSize size = mSurfaceKey.Size();
   return size.width * size.height * sizeof(uint32_t);
 }
 
 void
 DecodedSurfaceProvider::Run()
 {
+  MutexAutoLock lock(mMutex);
+
   if (!mDecoder || !mImage) {
     MOZ_ASSERT_UNREACHABLE("Running after decoding finished?");
     return;
   }
 
   // Run the decoder.
   LexerResult result = mDecoder->Decode(WrapNotNull(this));
 
@@ -157,16 +160,19 @@ DecodedSurfaceProvider::Run()
   MOZ_ASSERT_UNREACHABLE("Unexpected yield for single-frame image");
   mDecoder->TerminateFailure();
   FinishDecoding();
 }
 
 void
 DecodedSurfaceProvider::CheckForNewSurface()
 {
+  mMutex.AssertCurrentThreadOwns();
+  MOZ_ASSERT(mDecoder);
+
   if (mSurface) {
     // Single-frame images should produce no more than one surface, so if we
     // have one, it should be the same one the decoder is working on.
     MOZ_ASSERT(mSurface.get() == mDecoder->GetCurrentFrameRef().get(),
                "DecodedSurfaceProvider and Decoder have different surfaces?");
     return;
   }
 
@@ -181,16 +187,17 @@ DecodedSurfaceProvider::CheckForNewSurfa
   SurfaceCache::SurfaceAvailable(WrapNotNull(this),
                                  ImageKey(mImage.get()),
                                  mSurfaceKey);
 }
 
 void
 DecodedSurfaceProvider::FinishDecoding()
 {
+  mMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
 
   // Send notifications.
   NotifyDecodeComplete(WrapNotNull(mImage), WrapNotNull(mDecoder));
 
   // Destroy our decoder; we don't need it anymore. (And if we don't destroy it,
   // our surface can never be optimized, because the decoder has a
--- a/image/DecodedSurfaceProvider.h
+++ b/image/DecodedSurfaceProvider.h
@@ -65,16 +65,19 @@ private:
 
   void DropImageReference();
   void CheckForNewSurface();
   void FinishDecoding();
 
   /// The image associated with our decoder. Dropped after decoding.
   RefPtr<RasterImage> mImage;
 
+  /// Mutex protecting access to mDecoder.
+  Mutex mMutex;
+
   /// The decoder that will generate our surface. Dropped after decoding.
   RefPtr<Decoder> mDecoder;
 
   /// Our surface. Initially null until it's generated by the decoder.
   RefPtr<imgFrame> mSurface;
 
   /// A drawable reference to our service; used for locking.
   DrawableFrameRef mLockRef;
--- a/image/IDecodingTask.cpp
+++ b/image/IDecodingTask.cpp
@@ -100,27 +100,30 @@ IDecodingTask::Resume()
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // AnimationDecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
 AnimationDecodingTask::AnimationDecodingTask(NotNull<Decoder*> aDecoder)
-  : mDecoder(aDecoder)
+  : mMutex("mozilla::image::AnimationDecodingTask")
+  , mDecoder(aDecoder)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
   MOZ_ASSERT(!mDecoder->IsFirstFrameDecode(),
              "Use DecodingTask for single-frame image decodes");
 }
 
 void
 AnimationDecodingTask::Run()
 {
+  MutexAutoLock lock(mMutex);
+
   while (true) {
     LexerResult result = mDecoder->Decode(WrapNotNull(this));
 
     if (result.is<TerminalState>()) {
       NotifyDecodeComplete(mDecoder->GetImage(), mDecoder);
       return;  // We're done.
     }
 
--- a/image/IDecodingTask.h
+++ b/image/IDecodingTask.h
@@ -77,16 +77,19 @@ public:
 
   // Full decodes are low priority compared to metadata decodes because they
   // don't block layout or page load.
   TaskPriority Priority() const override { return TaskPriority::eLow; }
 
 private:
   virtual ~AnimationDecodingTask() { }
 
+  /// Mutex protecting access to mDecoder.
+  Mutex mMutex;
+
   NotNull<RefPtr<Decoder>> mDecoder;
 };
 
 
 /**
  * An IDecodingTask implementation for metadata decodes of images.
  */
 class MetadataDecodingTask final : public IDecodingTask