Bug 1069602 - Crash in mozilla::MediaOmxReader::NotifyDataArrived (ProcessCachedData runnable is running but reader has been destroyed). r=edwin, f=jwwang
authorRandy Lin <rlin@mozilla.com>
Thu, 02 Oct 2014 11:46:49 +0800
changeset 208707 d5290c0ca86ee534fb836a9b6255128a3bed22db
parent 208706 4e05fcdee48f8a54dc27ca4e11ec66e70896fa9a
child 208708 b1b5b6c55e8227afeebf373bf8279c64ce26f743
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersedwin
bugs1069602
milestone35.0a1
Bug 1069602 - Crash in mozilla::MediaOmxReader::NotifyDataArrived (ProcessCachedData runnable is running but reader has been destroyed). r=edwin, f=jwwang
content/media/omx/MediaOmxReader.cpp
content/media/omx/MediaOmxReader.h
--- a/content/media/omx/MediaOmxReader.cpp
+++ b/content/media/omx/MediaOmxReader.cpp
@@ -93,16 +93,19 @@ public:
     NotifyDataArrived();
 
     return NS_OK;
   }
 
 private:
   void NotifyDataArrived()
   {
+    if (mOmxReader->IsShutdown()) {
+      return;
+    }
     const char* buffer = mBuffer.get();
 
     while (mLength) {
       uint32_t length = std::min<uint64_t>(mLength, UINT32_MAX);
       mOmxReader->NotifyDataArrived(buffer, length,
                                     mOffset);
       buffer  += length;
       mLength -= length;
@@ -113,33 +116,42 @@ private:
       // We cannot read data in the main thread because it
       // might block for too long. Instead we post an IO task
       // to the IO thread if there is more data available.
       XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
           new OmxReaderProcessCachedDataTask(mOmxReader.get(), mOffset));
     }
   }
 
-  nsRefPtr<MediaOmxReader> mOmxReader;
+  nsRefPtr<MediaOmxReader>         mOmxReader;
   nsAutoArrayPtr<const char>       mBuffer;
   uint64_t                         mLength;
   int64_t                          mOffset;
   uint64_t                         mFullLength;
 };
 
+void MediaOmxReader::CancelProcessCachedData()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MutexAutoLock lock(mMutex);
+  mIsShutdown = true;
+}
+
 MediaOmxReader::MediaOmxReader(AbstractMediaDecoder *aDecoder)
   : MediaOmxCommonReader(aDecoder)
+  , mMutex("MediaOmxReader.Data")
   , mMP3FrameParser(-1)
   , mHasVideo(false)
   , mHasAudio(false)
   , mVideoSeekTimeUs(-1)
   , mAudioSeekTimeUs(-1)
   , mSkipCount(0)
   , mUseParserDuration(false)
   , mLastParserDuration(-1)
+  , mIsShutdown(false)
 {
 #ifdef PR_LOGGING
   if (!gMediaDecoderLog) {
     gMediaDecoderLog = PR_NewLogModule("MediaDecoder");
   }
 #endif
 
   mAudioChannel = dom::AudioChannelService::GetDefaultAudioChannel();
@@ -159,16 +171,20 @@ void MediaOmxReader::ReleaseDecoder()
   if (mOmxDecoder.get()) {
     mOmxDecoder->ReleaseDecoder();
   }
   mOmxDecoder.clear();
 }
 
 void MediaOmxReader::Shutdown()
 {
+  nsCOMPtr<nsIRunnable> cancelEvent =
+    NS_NewRunnableMethod(this, &MediaOmxReader::CancelProcessCachedData);
+  NS_DispatchToMainThread(cancelEvent);
+
   ReleaseMediaResources();
   nsCOMPtr<nsIRunnable> event =
     NS_NewRunnableMethod(this, &MediaOmxReader::ReleaseDecoder);
   NS_DispatchToMainThread(event);
 }
 
 bool MediaOmxReader::IsWaitingMediaResources()
 {
@@ -444,17 +460,19 @@ bool MediaOmxReader::DecodeVideoFrame(bo
   }
 
   return true;
 }
 
 void MediaOmxReader::NotifyDataArrived(const char* aBuffer, uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(NS_IsMainThread());
-
+  if (IsShutdown()) {
+    return;
+  }
   if (HasVideo()) {
     return;
   }
 
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
@@ -542,16 +560,20 @@ void MediaOmxReader::EnsureActive() {
     return;
   }
   DebugOnly<nsresult> result = mOmxDecoder->Play();
   NS_ASSERTION(result == NS_OK, "OmxDecoder should be in play state to continue decoding");
 }
 
 int64_t MediaOmxReader::ProcessCachedData(int64_t aOffset, bool aWaitForCompletion)
 {
+  // Could run on decoder thread or IO thread.
+  if (IsShutdown()) {
+    return -1;
+  }
   // We read data in chunks of 32 KiB. We can reduce this
   // value if media, such as sdcards, is too slow.
   // Because of SD card's slowness, need to keep sReadSize to small size.
   // See Bug 914870.
   static const int64_t sReadSize = 32 * 1024;
 
   NS_ASSERTION(!NS_IsMainThread(), "Should not be on main thread.");
 
--- a/content/media/omx/MediaOmxReader.h
+++ b/content/media/omx/MediaOmxReader.h
@@ -25,26 +25,29 @@ namespace mozilla {
 namespace dom {
   class TimeRanges;
 }
 
 class AbstractMediaDecoder;
 
 class MediaOmxReader : public MediaOmxCommonReader
 {
+  // This flag protect the mIsShutdown variable, that may access by decoder / main / IO thread.
+  Mutex mMutex;
   nsCString mType;
   bool mHasVideo;
   bool mHasAudio;
   nsIntRect mPicture;
   nsIntSize mInitialFrame;
   int64_t mVideoSeekTimeUs;
   int64_t mAudioSeekTimeUs;
   int64_t mLastParserDuration;
   int32_t mSkipCount;
   bool mUseParserDuration;
+  bool mIsShutdown;
 protected:
   android::sp<android::OmxDecoder> mOmxDecoder;
   android::sp<android::MediaExtractor> mExtractor;
   MP3FrameParser mMP3FrameParser;
   // Called by ReadMetadata() during MediaDecoderStateMachine::DecodeMetadata()
   // on decode thread. It create and initialize the OMX decoder including
   // setting up custom extractor. The extractor provide the essential
   // information used for creating OMX decoder such as video/audio codec.
@@ -86,18 +89,25 @@ public:
   virtual nsresult Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime);
 
   virtual bool IsMediaSeekable() MOZ_OVERRIDE;
 
   virtual void SetIdle() MOZ_OVERRIDE;
 
   virtual void Shutdown() MOZ_OVERRIDE;
 
+  bool IsShutdown() {
+    MutexAutoLock lock(mMutex);
+    return mIsShutdown;
+  }
+
   void ReleaseDecoder();
 
   int64_t ProcessCachedData(int64_t aOffset, bool aWaitForCompletion);
 
+  void CancelProcessCachedData();
+
   android::sp<android::MediaSource> GetAudioOffloadTrack();
 };
 
 } // namespace mozilla
 
 #endif