Bug 1394724. P2 - mListener->Revoke() should happen after mChannel->Cancel() to avoid data race. r=cpearce
authorJW Wang <jwwang@mozilla.com>
Tue, 29 Aug 2017 15:55:20 +0800
changeset 377826 4e9acbce2e29df707cf58881581dc094e9d425b1
parent 377825 a81e995c2a6a06447745f22f60fab62ca88f2eb5
child 377827 a91707ac4a3aad0200fd6d34bfb2d253c89661b2
push id50044
push userjwwang@mozilla.com
push dateThu, 31 Aug 2017 03:05:23 +0000
treeherderautoland@87c1afc748cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1394724
milestone57.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 1394724. P2 - mListener->Revoke() should happen after mChannel->Cancel() to avoid data race. r=cpearce http://searchfox.org/mozilla-central/rev/cd82cacec2cf734768827ff85ba2dba90a534c5e/netwerk/base/nsIRequest.idl#56 All OnDataAvailable() calls should happen before Cancel(). It is safe to modify mResource after a call to Cancel(). MozReview-Commit-ID: KoeLth1zlZM
dom/media/MediaResource.cpp
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -147,18 +147,18 @@ ChannelMediaResource::Listener::OnStopRe
 
 nsresult
 ChannelMediaResource::Listener::OnDataAvailable(nsIRequest* aRequest,
                                                 nsISupports* aContext,
                                                 nsIInputStream* aStream,
                                                 uint64_t aOffset,
                                                 uint32_t aCount)
 {
-  if (!mResource)
-    return NS_OK;
+  // This might happen off the main thread.
+  MOZ_DIAGNOSTIC_ASSERT(mResource);
   return mResource->OnDataAvailable(aRequest, aStream, aCount);
 }
 
 nsresult
 ChannelMediaResource::Listener::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
                                                        nsIChannel* aNewChannel,
                                                        uint32_t aFlags,
                                                        nsIAsyncVerifyRedirectCallback* cb)
@@ -676,33 +676,33 @@ void ChannelMediaResource::CloseChannel(
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   {
     MutexAutoLock lock(mLock);
     mChannelStatistics.Stop();
   }
 
-  if (mListener) {
-    mListener->Revoke();
-    mListener = nullptr;
-  }
-
   if (mChannel) {
     mSuspendAgent.NotifyChannelClosing();
     // The status we use here won't be passed to the decoder, since
     // we've already revoked the listener. It can however be passed
     // to nsDocumentViewer::LoadComplete if our channel is the one
     // that kicked off creation of a video document. We don't want that
     // document load to think there was an error.
     // NS_ERROR_PARSED_DATA_CACHED is the best thing we have for that
     // at the moment.
     mChannel->Cancel(NS_ERROR_PARSED_DATA_CACHED);
     mChannel = nullptr;
   }
+
+  if (mListener) {
+    mListener->Revoke();
+    mListener = nullptr;
+  }
 }
 
 nsresult ChannelMediaResource::ReadFromCache(char* aBuffer,
                                              int64_t aOffset,
                                              uint32_t aCount)
 {
   return mCacheStream.ReadFromCache(aBuffer, aOffset, aCount);
 }