Closed Bug 975858 Opened 6 years ago Closed 6 years ago
Resource still isn't safe to release on non-main threads
In bug 929029 we tried to make MediaResource safe to release on non-main threads. We missed a case; ChannelMediaResource is composed of a MediaCacheStream, and that asserts that it should be released on the main thread and interacts with the global mediacache instance assuming it's running on the main thread. So it's not threadsafe. I could reproduce a crash in a Windows Opt build when looping over lots of MP4 test cases in test_seek with PARALLEL_TESTS=20, which I tracked down to this. I propose we fix this once and for all by making MediaResource::Release() detect when the refcnt drops to 0 and dispatching an event to the main thread to delete the MediaResource. So we ensure the dtor always runs on the main thread.
When the refcount of a MediaResource drops to 0, dispatch an event to the main thread to delete the resource.
Attachment #8380380 - Flags: review?(roc)
Comment on attachment 8380380 [details] [diff] [review] Patch: Always delete MediaResource on the main thread Review of attachment 8380380 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/MediaResource.cpp @@ +75,5 @@ > + // set to 1 by refcount stabilization by Release() calling us, and the > + // refcount should be incremented by ReleaseOnMainThread's ref ptr. > + MOZ_ASSERT(aResource->mRefCnt == 2); > + NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); > + event = nullptr; I think you can just use NS_ProxyRelease.
Attachment #8380380 - Flags: review?(roc) → review-
Using NS_ProxyRelease. I had to also make MediaResource inherit from nsISupports, but it was already refcounted, so no big deal.
Err, I meant: https://tbpl.mozilla.org/?tree=Try&rev=30bd8bcbd8ac
Attachment #8380950 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.