Closed Bug 483324 Opened 13 years ago Closed 13 years ago
Media Stream::Close called on non-main thread" with Load Ogg Headers on the stack
I'm hitting this on mozilla-central a lot but haven't been able to make a testcase. ###!!! ASSERTION: nsMediaStream::Close called on non-main thread: 'NS_IsMainThread()', file /Users/jruderman/central/content/media/video/src/nsMediaStream.cpp, line 845 nsMediaStream::Close() nsChannelReader::destroy() oggplay_channel_reader_destroy(_OggPlayReader*) oggplay_close oggplay_open_with_reader nsOggDecodeStateMachine::LoadOggHeaders(nsChannelReader*) nsOggDecodeStateMachine::Run() nsThread::ProcessNextEvent(int, int*) NS_ProcessNextEvent_P(nsIThread*, int) nsThread::ThreadFunc(void*) _pt_root _pthread_start thread_start Similar to the fixed bug 462878?
Often followed by additional thread-safety assertions, such as ###!!! ASSERTION: Only call on main thread: 'NS_IsMainThread()', file /Users/jruderman/central/content/media/video/src/nsMediaStream.cpp, line 564 ###!!! ASSERTION: nsStandardURL not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/jruderman/central/netwerk/base/src/nsStandardURL.cpp, line 872 ###!!! ASSERTION: nsVariant not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/jruderman/central/xpcom/ds/nsVariant.cpp, line 1694
These assertions are firing thanks to the new version of liboggplay which we updated last week or so. oggplay_open_with_reader() used to call free() on failure, now it's calling oggplay_close() on failure, which is calling into our code and so the assertions are being triggered.
Duplicate of this bug: 483563
In nsHTMLMediaElement::MediaLoadListener::OnStartRequest(), we always try to initialized a channel. However when an access controls check has failed, nsCrossSiteListenerProxy will Cancel() the channel, but we still load a nsMediaDecoder, and create a decode state machine/thread. Then the channel closes, but we try to read it in LoadOggHeaders(), and that fails, which now calls oggplay_close(), hitting the assertion. MediaLoadListener::OnStartRequest() should not try to load the channel if it's already been canceled. Eventually the call path in MediaLoadListener::OnStartRequest() calls down to nsChannelToPipeListener::OnStartRequest() which checks the request status and aborts the load (again). We should just bail out in MediaLoadListener::OnStartRequest() if the channel is canceled.
Ah, I think I discovered and fixed this in my patch in bug 475441 by allowing nsMediaStream::Close to happen on non-main threads. But we should do what you suggest as well.
As per comment 4, don't try to load in MediaLoadListener::OnStartRequest() if the request is already canceled. The nsMediaStream::Close() assertion failures go away, and all video Mochitest still pass on Windows. I'm updating my Linux build now, and will report back if any of its Mochitests fail.
Comment on attachment 367658 [details] [diff] [review] Patch v1 + // Don't continue to load load if the request failed or has been canceled. "load load"?
As v1 with typo fixed. r+/sr+ roc.
Whiteboard: [needs landing]
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Attachment #367659 - Flags: approval1.9.1+
Whiteboard: [needs 191 landing]
You need to log in before you can comment on or make changes to this bug.