Bug 1401461. P1 - protect access to ChannelMediaResource::Listener::mResource. draft
authorJW Wang <jwwang@mozilla.com>
Wed, 20 Sep 2017 14:37:18 +0800
changeset 668292 f4d3f875d3c62e8243e56efec14828070d3e3dcf
parent 668025 47f7b6c64265bc7bdd22eef7ab71abc97cf3f8bf
child 668293 b05a90254c021dc899f35e9d14afc9d83923987a
push id81004
push userjwwang@mozilla.com
push dateThu, 21 Sep 2017 13:50:51 +0000
bugs1401461
milestone57.0a1
Bug 1401461. P1 - protect access to ChannelMediaResource::Listener::mResource. MozReview-Commit-ID: 6G1x7cXNvAq
dom/media/ChannelMediaResource.cpp
dom/media/ChannelMediaResource.h
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -63,50 +63,60 @@ NS_IMPL_ISUPPORTS(ChannelMediaResource::
                   nsIChannelEventSink,
                   nsIInterfaceRequestor,
                   nsIThreadRetargetableStreamListener)
 
 nsresult
 ChannelMediaResource::Listener::OnStartRequest(nsIRequest* aRequest,
                                                nsISupports* aContext)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   if (!mResource)
     return NS_OK;
   return mResource->OnStartRequest(aRequest, mOffset);
 }
 
 nsresult
 ChannelMediaResource::Listener::OnStopRequest(nsIRequest* aRequest,
                                               nsISupports* aContext,
                                               nsresult aStatus)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   if (!mResource)
     return NS_OK;
   return mResource->OnStopRequest(aRequest, aStatus);
 }
 
 nsresult
 ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest,
                                                 nsISupports* aContext,
                                                 nsIInputStream* aStream,
                                                 uint64_t aOffset,
                                                 uint32_t aCount)
 {
   // This might happen off the main thread.
-  MOZ_DIAGNOSTIC_ASSERT(mResource);
-  return mResource->OnDataAvailable(mLoadID, aStream, aCount);
+  RefPtr<ChannelMediaResource> res;
+  {
+    MutexAutoLock lock(mMutex);
+    res = mResource;
+  }
+  // Note Rekove() might happen at the same time to reset mResource. We check
+  // the load ID to determine if the data is from an old channel.
+  return res ? res->OnDataAvailable(mLoadID, aStream, aCount) : NS_OK;
 }
 
 nsresult
 ChannelMediaResource::Listener::AsyncOnChannelRedirect(
   nsIChannel* aOld,
   nsIChannel* aNew,
   uint32_t aFlags,
   nsIAsyncVerifyRedirectCallback* cb)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   nsresult rv = NS_OK;
   if (mResource) {
     rv = mResource->OnChannelRedirect(aOld, aNew, aFlags, mOffset);
   }
 
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -122,16 +132,24 @@ ChannelMediaResource::Listener::CheckLis
 }
 
 nsresult
 ChannelMediaResource::Listener::GetInterface(const nsIID& aIID, void** aResult)
 {
   return QueryInterface(aIID, aResult);
 }
 
+void
+ChannelMediaResource::Listener::Revoke()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MutexAutoLock lock(mMutex);
+  mResource = nullptr;
+}
+
 static bool
 IsPayloadCompressed(nsIHttpChannel* aChannel)
 {
   nsAutoCString encoding;
   Unused << aChannel->GetResponseHeader(NS_LITERAL_CSTRING("Content-Encoding"), encoding);
   return encoding.Length() > 0;
 }
 
@@ -410,18 +428,16 @@ ChannelMediaResource::CopySegmentToCache
 }
 
 nsresult
 ChannelMediaResource::OnDataAvailable(uint32_t aLoadID,
                                       nsIInputStream* aStream,
                                       uint32_t aCount)
 {
   // This might happen off the main thread.
-  // Don't assert |mChannel.get() == aRequest| since reading mChannel here off
-  // the main thread is a data race.
 
   RefPtr<ChannelMediaResource> self = this;
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
     "ChannelMediaResource::OnDataAvailable",
     [self, aCount]() { self->mChannelStatistics.AddBytes(aCount); });
   mCallback->AbstractMainThread()->Dispatch(r.forget());
 
   Closure closure{ aLoadID, this };
--- a/dom/media/ChannelMediaResource.h
+++ b/dom/media/ChannelMediaResource.h
@@ -111,31 +111,36 @@ public:
     : public nsIStreamListener
     , public nsIInterfaceRequestor
     , public nsIChannelEventSink
     , public nsIThreadRetargetableStreamListener
   {
     ~Listener() {}
   public:
     Listener(ChannelMediaResource* aResource, int64_t aOffset, uint32_t aLoadID)
-      : mResource(aResource)
+      : mMutex("Listener.mMutex")
+      , mResource(aResource)
       , mOffset(aOffset)
       , mLoadID(aLoadID)
     {}
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIREQUESTOBSERVER
     NS_DECL_NSISTREAMLISTENER
     NS_DECL_NSICHANNELEVENTSINK
     NS_DECL_NSIINTERFACEREQUESTOR
     NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
 
-    void Revoke() { mResource = nullptr; }
+    void Revoke();
 
   private:
+    Mutex mMutex;
+    // mResource should only be modified on the main thread with the lock.
+    // So it can be read without lock on the main thread or on other threads
+    // with the lock.
     RefPtr<ChannelMediaResource> mResource;
     const int64_t mOffset;
     const uint32_t mLoadID;
   };
   friend class Listener;
 
   nsresult GetCachedRanges(MediaByteRangeSet& aRanges) override;