Closed
Bug 1196696
Opened 9 years ago
Closed 9 years ago
crash in mozilla::MediaFormatReader::DecoderData::ResetDemuxer()
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox40 | --- | unaffected |
firefox41 | + | fixed |
firefox42 | --- | fixed |
firefox43 | --- | fixed |
People
(Reporter: philipp, Assigned: jya)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
3.72 KB,
patch
|
jwwang
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-e1c75185-45f3-483c-b44d-3c5612150818. ============================================================= Crashing Thread Frame Module Signature Source 0 xul.dll mozilla::MediaFormatReader::DecoderData::ResetDemuxer() dom/media/MediaFormatReader.h 1 xul.dll mozilla::MediaFormatReader::ResetDecode() dom/media/MediaFormatReader.cpp 2 xul.dll mozilla::MediaSourceReader::ResetDecode() dom/media/mediasource/MediaSourceReader.cpp 3 xul.dll nsRunnableMethodImpl<void ( mozilla::dom::indexedDB::`anonymous namespace'::DatabaseOfflineStorage::*)(void), 1>::Run() xpcom/glue/nsThreadUtils.h 4 xul.dll mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() dom/media/TaskDispatcher.h 5 xul.dll mozilla::MediaTaskQueue::Runner::Run() dom/media/MediaTaskQueue.cpp 6 xul.dll nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp 7 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp 8 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp 9 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp 10 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc 11 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc 12 xul.dll nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp 13 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c 14 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c 15 msvcr120.dll _callthreadstartex f:\dd\vctools\crt\crtw32\startup\threadex.c:376 16 msvcr120.dll msvcr120.dll@0x2c000 17 kernel32.dll BaseThreadInitThunk 18 ntdll.dll __RtlUserThreadStart 19 ntdll.dll _RtlUserThreadStart according to crash stats this crash shows up in firefox 41 builds on windows, but not in prior or subsequent versions. possibly related to http://hg.mozilla.org/releases/mozilla-beta/diff/5cf8087b6812/dom/media/MediaFormatReader.h and the landed code in bug 1163227?
Assignee | ||
Comment 1•9 years ago
|
||
And another crash that makes no sense. A null deref, but if HasAudio() returned true, then the audio demuxer demuxer must exist and not be null!
Assignee: nobody → jyavenard
Assignee | ||
Comment 2•9 years ago
|
||
HasAudio() returns true if mInfo.HasAudio() returns true. mInfo.mAudio is created by the track demuxer, so for mInfo.HasAudio() to return true, mAudio.mDemuxer will always be set. The only time in the lifetime of the MediaFormatReader that a demuxer is null, is after shutdown.. So that would mean MediaDecoderReader::ResetDecode() is called after Shutdown. Chris, do you see a way this could be happening?
Flags: needinfo?(cpearce)
Comment 4•9 years ago
|
||
All seems unlikely to me: 1. ResetDecode() called after Shutdown 2. https://hg.mozilla.org/mozilla-central/file/29b2df16e961/dom/media/MediaFormatReader.cpp#l1208 mInfo.mAudio.mRate or mInfo.mAudio.mChannels are tampered so HasAudio() becomes true 3. memory overwrite at random position. How is it reproducible?
Flags: needinfo?(jwwang)
Tracked for FF41 as this was mentioned in the channel meeting today, we are hitting this crash at ~1%
tracking-firefox41:
--- → +
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8652671 -
Flags: review?(jwwang)
Updated•9 years ago
|
Attachment #8652671 -
Flags: review?(jwwang) → review+
Assignee | ||
Comment 8•9 years ago
|
||
I still don't see how the crash could ever happen; the patch will get around the null deref ; but if we get in ResetDecode() after Shutdown() then it will continue to cause problem (but elsewhere)
Flags: needinfo?(cpearce)
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/28fb66c200e4
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8652671 [details] [diff] [review] Always check that track demuxer was successfully created. Approval Request Comment [Feature/regressing bug #]: 1171379 [User impact if declined]: Crashes. [Describe test coverage new/current, TreeHerder]: local test [Risks and why]: Very low. Used to assert, now returns an error [String/UUID change made/needed]: None
Attachment #8652671 -
Flags: approval-mozilla-beta?
Attachment #8652671 -
Flags: approval-mozilla-aurora?
Comment 11•9 years ago
|
||
I see a single crash with 42 when it was in nightly. I guess this version is indeed affected (and will probably come back in 42 beta if we don't take the patch).
Flags: needinfo?(jyavenard)
Comment 12•9 years ago
|
||
Comment on attachment 8652671 [details] [diff] [review] Always check that track demuxer was successfully created. Fix a crash, taking it.
Attachment #8652671 -
Flags: approval-mozilla-beta?
Attachment #8652671 -
Flags: approval-mozilla-beta+
Attachment #8652671 -
Flags: approval-mozilla-aurora?
Attachment #8652671 -
Flags: approval-mozilla-aurora+
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(jyavenard)
You need to log in
before you can comment on or make changes to this bug.
Description
•