Bug 1293577 - Protect the image decoding task path with a mutex to av…

…oid race conditions. r=seth
iaxat · Aug 10, 2016 · 1103bff · 1103bff
1 parent 0a75260
commit 1103bff
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 1 deletion.
diff --git a/image/DecodedSurfaceProvider.cpp b/image/DecodedSurfaceProvider.cpp
@@ -20,6 +20,7 @@ DecodedSurfaceProvider::DecodedSurfaceProvider(NotNull<RasterImage*> aImage,
                                                const SurfaceKey& aSurfaceKey)
   : ISurfaceProvider(AvailabilityState::StartAsPlaceholder())
   , mImage(aImage.get())
+  , mMutex("mozilla::image::DecodedSurfaceProvider")
   , mDecoder(aDecoder.get())
   , mSurfaceKey(aSurfaceKey)
 {
@@ -124,6 +125,8 @@ DecodedSurfaceProvider::LogicalSizeInBytes() const
 void
 DecodedSurfaceProvider::Run()
 {
+  MutexAutoLock lock(mMutex);
+
   if (!mDecoder || !mImage) {
     MOZ_ASSERT_UNREACHABLE("Running after decoding finished?");
     return;
@@ -162,6 +165,9 @@ DecodedSurfaceProvider::Run()
 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.
@@ -186,6 +192,7 @@ DecodedSurfaceProvider::CheckForNewSurface()
 void
 DecodedSurfaceProvider::FinishDecoding()
 {
+  mMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
 

diff --git a/image/DecodedSurfaceProvider.h b/image/DecodedSurfaceProvider.h
@@ -70,6 +70,9 @@ class DecodedSurfaceProvider final
   /// 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;
 

diff --git a/image/IDecodingTask.cpp b/image/IDecodingTask.cpp
@@ -105,7 +105,8 @@ IDecodingTask::Resume()
 ///////////////////////////////////////////////////////////////////////////////
 
 AnimationDecodingTask::AnimationDecodingTask(NotNull<Decoder*> aDecoder)
-  : mDecoder(aDecoder)
+  : mMutex("mozilla::image::AnimationDecodingTask")
+  , mDecoder(aDecoder)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
@@ -116,6 +117,8 @@ AnimationDecodingTask::AnimationDecodingTask(NotNull<Decoder*> aDecoder)
 void
 AnimationDecodingTask::Run()
 {
+  MutexAutoLock lock(mMutex);
+
   while (true) {
     LexerResult result = mDecoder->Decode(WrapNotNull(this));
 

diff --git a/image/IDecodingTask.h b/image/IDecodingTask.h
@@ -82,6 +82,9 @@ class AnimationDecodingTask final : public IDecodingTask
 private:
   virtual ~AnimationDecodingTask() { }
 
+  /// Mutex protecting access to mDecoder.
+  Mutex mMutex;
+
   NotNull<RefPtr<Decoder>> mDecoder;
 };