Commit 0d13381c authored by alwu's avatar alwu Committed by Richard Pospesel
Browse files

Bug 1644383 - add mutexs to avoid data race. r=media-playback-reviewers,padenot 

Differential Revision: https://phabricator.services.mozilla.com/D206943
parent a04f2066

dom/media/ipc/RemoteMediaDataDecoder.cpp

+15 −2
Original line number Diff line number Diff line
@@ -18,7 +18,12 @@ namespace mozilla { 
            ##__VA_ARGS__)



RemoteMediaDataDecoder::RemoteMediaDataDecoder(RemoteDecoderChild* aChild)

    : mChild(aChild) {

    : mChild(aChild),

      mDescription("RemoteMediaDataDecoder"_ns),

      mProcessName("unknown"_ns),

      mCodecName("unknown"_ns),

      mIsHardwareAccelerated(false),

      mConversion(ConversionRequired::kNeedNone) {

  LOG("%p is created", this);

}
@@ -48,6 +53,7 @@ RefPtr<MediaDataDecoder::InitPromise> RemoteMediaDataDecoder::Init() { 
      ->Then(

          RemoteDecoderManagerChild::GetManagerThread(), __func__,

          [self, this](TrackType aTrack) {

            MutexAutoLock lock(mMutex);

            // If shutdown has started in the meantime shutdown promise may

            // be resloved before this task. In this case mChild will be null

            // and the init promise has to be canceled.
@@ -127,6 +133,7 @@ RefPtr<ShutdownPromise> RemoteMediaDataDecoder::Shutdown() { 


bool RemoteMediaDataDecoder::IsHardwareAccelerated(

    nsACString& aFailureReason) const {

  MutexAutoLock lock(mMutex);

  aFailureReason = mHardwareAcceleratedReason;

  return mIsHardwareAccelerated;

}
@@ -145,18 +152,24 @@ void RemoteMediaDataDecoder::SetSeekThreshold(const media::TimeUnit& aTime) { 


MediaDataDecoder::ConversionRequired RemoteMediaDataDecoder::NeedsConversion()

    const {

  MutexAutoLock lock(mMutex);

  return mConversion;

}



nsCString RemoteMediaDataDecoder::GetDescriptionName() const {

  MutexAutoLock lock(mMutex);

  return mDescription;

}



nsCString RemoteMediaDataDecoder::GetProcessName() const {

  MutexAutoLock lock(mMutex);

  return mProcessName;

}



nsCString RemoteMediaDataDecoder::GetCodecName() const { return mCodecName; }

nsCString RemoteMediaDataDecoder::GetCodecName() const {

  MutexAutoLock lock(mMutex);

  return mCodecName;

}



#undef LOG

dom/media/ipc/RemoteMediaDataDecoder.h

+9 −7
Original line number Diff line number Diff line
@@ -53,14 +53,16 @@ class RemoteMediaDataDecoder final 
  // destructor when we can guarantee no other threads are accessing it). Only

  // read from the manager thread.

  RefPtr<RemoteDecoderChild> mChild;



  mutable Mutex mMutex{"RemoteMediaDataDecoder"};



  // Only ever written/modified during decoder initialisation.

  // As such can be accessed from any threads after that.

  nsCString mDescription = "RemoteMediaDataDecoder"_ns;

  nsCString mProcessName = "unknown"_ns;

  nsCString mCodecName = "unknown"_ns;

  bool mIsHardwareAccelerated = false;

  nsCString mHardwareAcceleratedReason;

  ConversionRequired mConversion = ConversionRequired::kNeedNone;

  nsCString mDescription MOZ_GUARDED_BY(mMutex);

  nsCString mProcessName MOZ_GUARDED_BY(mMutex);

  nsCString mCodecName MOZ_GUARDED_BY(mMutex);

  bool mIsHardwareAccelerated MOZ_GUARDED_BY(mMutex);

  nsCString mHardwareAcceleratedReason MOZ_GUARDED_BY(mMutex);

  ConversionRequired mConversion MOZ_GUARDED_BY(mMutex);

};



}  // namespace mozilla

dom/media/platforms/wrappers/MediaChangeMonitor.cpp

+7 −0
Original line number Diff line number Diff line
@@ -668,6 +668,7 @@ RefPtr<ShutdownPromise> MediaChangeMonitor::ShutdownDecoder() { 
  AssertOnThread();

  mConversionRequired.reset();

  if (mDecoder) {

    MutexAutoLock lock(mMutex);

    RefPtr<MediaDataDecoder> decoder = std::move(mDecoder);

    return decoder->Shutdown();

  }
@@ -715,6 +716,7 @@ MediaChangeMonitor::CreateDecoder() { 
          ->Then(

              GetCurrentSerialEventTarget(), __func__,

              [self = RefPtr{this}, this](RefPtr<MediaDataDecoder>&& aDecoder) {

                MutexAutoLock lock(mMutex);

                mDecoder = std::move(aDecoder);

                DDLINKCHILD("decoder", mDecoder.get());

                return CreateDecoderPromise::CreateAndResolve(true, __func__);
@@ -963,6 +965,11 @@ void MediaChangeMonitor::FlushThenShutdownDecoder( 
      ->Track(mFlushRequest);

}



MediaDataDecoder* MediaChangeMonitor::GetDecoderOnNonOwnerThread() const {

  MutexAutoLock lock(mMutex);

  return mDecoder;

}



#undef LOG



}  // namespace mozilla

dom/media/platforms/wrappers/MediaChangeMonitor.h

+20 −10
Original line number Diff line number Diff line
@@ -41,34 +41,34 @@ class MediaChangeMonitor final 
  RefPtr<ShutdownPromise> Shutdown() override;

  bool IsHardwareAccelerated(nsACString& aFailureReason) const override;

  nsCString GetDescriptionName() const override {

    if (mDecoder) {

      return mDecoder->GetDescriptionName();

    if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) {

      return decoder->GetDescriptionName();

    }

    return "MediaChangeMonitor decoder (pending)"_ns;

  }

  nsCString GetProcessName() const override {

    if (mDecoder) {

      return mDecoder->GetProcessName();

    if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) {

      return decoder->GetProcessName();

    }

    return "MediaChangeMonitor"_ns;

  }

  nsCString GetCodecName() const override {

    if (mDecoder) {

      return mDecoder->GetCodecName();

    if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) {

      return decoder->GetCodecName();

    }

    return "MediaChangeMonitor"_ns;

  }

  void SetSeekThreshold(const media::TimeUnit& aTime) override;

  bool SupportDecoderRecycling() const override {

    if (mDecoder) {

      return mDecoder->SupportDecoderRecycling();

    if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) {

      return decoder->SupportDecoderRecycling();

    }

    return false;

  }



  ConversionRequired NeedsConversion() const override {

    if (mDecoder) {

      return mDecoder->NeedsConversion();

    if (RefPtr<MediaDataDecoder> decoder = GetDecoderOnNonOwnerThread()) {

      return decoder->NeedsConversion();

    }

    // Default so no conversion is performed.

    return ConversionRequired::kNeedNone;
@@ -97,6 +97,9 @@ class MediaChangeMonitor final 
    MOZ_ASSERT(!mThread || mThread->IsOnCurrentThread());

  }



  // This is used for getting decoder debug info on other threads. Thread-safe.

  MediaDataDecoder* GetDecoderOnNonOwnerThread() const;



  bool CanRecycleDecoder() const;



  typedef MozPromise<bool, MediaResult, true /* exclusive */>
@@ -137,6 +140,13 @@ class MediaChangeMonitor final 
  const CreateDecoderParamsForAsync mParams;

  // Keep any seek threshold set for after decoder creation and initialization.

  Maybe<media::TimeUnit> mPendingSeekThreshold;



  // This lock is used for mDecoder specifically, but it doens't need to be used

  // for every places accessing mDecoder which is mostly on the owner thread.

  // However, when requesting decoder debug info, it can happen on other

  // threads, so we need this mutex to avoid the data race of

  // creating/destroying decoder and accessing decoder's debug info.

  mutable Mutex MOZ_ANNOTATED mMutex{"MediaChangeMonitor"};

};



}  // namespace mozilla