Bug 1394724. P3 - fix data race in reading mClosed in MediaCacheStream::NotifyDataReceived().
1. mCacheStream.Close() should happen after CloseChannel() to avoid data race
in reading mClosed in MediaCacheStream::NotifyDataReceived().
2. MediaCache::Update() and CloseStreamsForPrivateBrowsing() should call
ChannelMediaResource::Close() to ensure mCacheStream.Close() happens
after CloseChannel().
MozReview-Commit-ID: 1o3yPbm3Gy6
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -670,17 +670,17 @@ MediaCache::Flush()
}
void
MediaCache::CloseStreamsForPrivateBrowsing()
{
MOZ_ASSERT(NS_IsMainThread());
for (MediaCacheStream* s : mStreams) {
if (s->mIsPrivateBrowsing) {
- s->Close();
+ s->mClient->Close();
}
}
}
/* static */ RefPtr<MediaCache>
MediaCache::GetMediaCache(int64_t aContentLength)
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
@@ -1454,18 +1454,17 @@ MediaCache::Update()
rv = NS_OK;
break;
}
if (NS_FAILED(rv)) {
// Close the streams that failed due to error. This will cause all
// client Read and Seek operations on those streams to fail. Blocked
// Reads will also be woken up.
- ReentrantMonitorAutoEnter mon(mReentrantMonitor);
- stream->CloseInternal(mon);
+ stream->mClient->Close();
}
}
// Notify streams about the suspended status changes.
for (uint32_t i = 0; i < mSuspendedStatusToNotify.Length(); ++i) {
MediaCache::ResourceStreamIterator iter(this, mSuspendedStatusToNotify[i]);
while (MediaCacheStream* stream = iter.Next()) {
stream->mClient->CacheClientNotifySuspendedStatusChanged();
@@ -1875,21 +1874,18 @@ MediaCacheStream::UpdatePrincipal(nsIPri
{
return nsContentUtils::CombineResourcePrincipals(&mPrincipal, aPrincipal);
}
void
MediaCacheStream::NotifyDataReceived(int64_t aSize, const char* aData,
nsIPrincipal* aPrincipal)
{
- NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
-
- if (mClosed) {
- return;
- }
+ // This might happen off the main thread.
+ MOZ_DIAGNOSTIC_ASSERT(!mClosed);
// Update principals before putting the data in the cache. This is important,
// we want to make sure all principals are updated before any consumer
// can see the new data.
// We do this without holding the cache monitor, in case the client wants
// to do something that takes a lock.
{
MediaCache::ResourceStreamIterator iter(mMediaCache, mResourceID);
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -627,18 +627,18 @@ nsresult ChannelMediaResource::SetupChan
}
return NS_OK;
}
nsresult ChannelMediaResource::Close()
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
+ CloseChannel();
mCacheStream.Close();
- CloseChannel();
return NS_OK;
}
already_AddRefed<nsIPrincipal> ChannelMediaResource::GetCurrentPrincipal()
{
NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
nsCOMPtr<nsIPrincipal> principal = mCacheStream.GetCurrentPrincipal();