Closed
Bug 1093567
Opened 10 years ago
Closed 10 years ago
Assertion failure: aIndex < Length() (invalid array index), at c:\users\mozilla\debug-builds\mozilla-central\firefox-debug\dist\include\nsTArray.h:936
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: cbook, Assigned: rillian)
References
()
Details
(Keywords: assertion)
Attachments
(3 files, 4 obsolete files)
6.09 KB,
text/plain
|
Details | |
2.63 KB,
patch
|
rillian
:
review+
|
Details | Diff | Splinter Review |
2.57 KB,
patch
|
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Found via Bughunter and reproduced on win7 trunk debug build Steps to reproduce: -> Load http://www.rainmakingloft.com/ --> Assertion failure: aIndex < Length() (invalid array index), at c:\users\mozilla\debug-builds\mozilla-central\firefox-debug\dist\include\nsTArray.h:936 STACK_TEXT: 02a9f730 658b6c26 ffffffff 00000000 0ba4c908 xul!nsTArray_Impl<int,nsTArrayInfallibleAllocator>::ElementAt+0x26 02a9f774 658c6453 12bf32e8 ce850040 00000002 xul!mozilla::MediaCache::NoteSeek+0x14f 02a9f79c 658be566 00000000 801f9652 00016742 xul!mozilla::MediaCacheStream::Seek+0xfe 02a9f7bc 658be42d 801f9652 00016742 02a9f854 xul!mozilla::MediaCacheStream::ReadAt+0x4f 02a9f7e4 6596037a 801f9652 00016742 02a9f854 xul!mozilla::ChannelMediaResource::ReadAt+0x4a 02a9f810 646973cd 801f9652 00016742 02a9f854 xul!mozilla::MP4Stream::ReadAt+0x32 02a9f830 6469241b 801f9652 00016742 02a9f854 xul!mp4_demuxer::DataSourceAdapter::readAt+0x56 02a9fab0 6469749c 02a9fae4 00000000 00000000 xul!stagefright::MPEG4Extractor::parseChunk+0x5b 02a9fafc 6469011a 11cfa970 6468d831 00000000 xul!stagefright::MPEG4Extractor::readMetaData+0x35 02a9fb04 6468d831 00000000 12a28c10 12a29400 xul!stagefright::MPEG4Extractor::countTracks+0x8 02a9fb54 659603d3 12a29400 12a28c10 12a28c00 xul!mp4_demuxer::MP4Demuxer::Init+0x36 02a9fb94 658aaa72 02a9fbb0 12a28c10 091aecc0 xul!mozilla::MP4Reader::ReadMetadata+0x32 02a9fc70 658a7aeb 091aecc0 091b0da0 122648b4 xul!mozilla::MediaDecoderStateMachine::DecodeMetadata+0x12f 02a9fcac 658c33e5 02a9fcdc 658c4f70 091b0da0 xul!mozilla::MediaDecoderStateMachine::CallDecodeMetadata+0x3d 02a9fcb4 658c4f70 091b0da0 127bc204 127bc214 xul!nsRunnableMethodImpl<void (__thiscall mozilla::MediaDecoderStateMachine::*)(void),void,1>::Run+0x10 02a9fcdc 647168a8 091aecc0 0de93701 0de93740 xul!mozilla::MediaTaskQueue::Runner::Run+0xd0 02a9fd14 64714ad0 00a9fd04 12bf3358 12bf3340 xul!nsThreadPool::Run+0x1d6 02a9fdf0 6473c81b 127bc204 00000001 02a9fe0f xul!nsThread::ProcessNextEvent+0x3a0 02a9fe04 64988fdc 01e93740 00000001 12a3d2c0 xul!NS_ProcessNextEvent+0x46 02a9fe30 6495abcc 00a3d2c0 12a3d2c0 12bf3340 xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0x16b 02a9fe50 6495ab84 e08b40a1 0de93740 12a3d2c0 xul!MessageLoop::RunInternal+0x42 02a9fe84 6495a901 12a3d2c0 00000001 02a9fe00 xul!MessageLoop::RunHandler+0x50 02a9fea4 6471854b 12bb69d0 12bb6920 01b16b90 xul!MessageLoop::Run+0x19 02a9fec4 67fb8ff8 12a3d2c0 01406350 01406350 xul!nsThread::ThreadFunc+0xcb 02a9fee0 67fa9c7f 12bf3310 02a9ff24 6ab9db10 nss3!_PR_NativeRunThread+0xc8 02a9feec 6ab9db10 12bb6920 ff2b42a0 00000000 nss3!pr_root+0xf 02a9ff24 6ab9daf4 00000000 02a9ff3c 778eed6c MSVCR110!_beginthreadex+0xb4 02a9ff30 778eed6c 01b16b90 02a9ff7c 77d2377b MSVCR110!_endthreadex+0x102 02a9ff3c 77d2377b 01b16b90 75c0fc75 00000000 kernel32!BaseThreadInitThunk+0xe 02a9ff7c 77d2374e 6ab9dab5 01b16b90 00000000 ntdll!__RtlUserThreadStart+0x70 02a9ff94 00000000 6ab9dab5 01b16b90 00000000 ntdll!_RtlUserThreadStart+0x1b
Reporter | ||
Comment 1•10 years ago
|
||
Hi Chris, padenot told me that maybe your area (or you might know whats going on here)
Flags: needinfo?(cpearce)
Comment 2•10 years ago
|
||
We've got a recent orange in the WebRTC tests with the same assertion, in case that's relevant.
See Also: → 1094969
Comment 3•10 years ago
|
||
I can repro this crash on Windows in Nightly simply by loading http://www.rainmakingloft.com/assets/media/rain_london.mp4 This is a crash in the mp4 demuxer's reading code. It looks like we don't find any valid tracks? WMF fails to parse that file, Release/WMFReader thinks that video is corrupt. Not even VLC can play that file. We'll want to either fix this on beta, or disable the MP4Reader on Beta.
Flags: needinfo?(cpearce) → needinfo?(ajones)
Comment 4•10 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #2) > We've got a recent orange in the WebRTC tests with the same assertion, in > case that's relevant. This has failure has a different cause with the same symptom. Not related.
See Also: 1094969 →
Comment 5•10 years ago
|
||
Ralph - do you have time to look into this?
Flags: needinfo?(ajones) → needinfo?(giles)
Assignee | ||
Comment 6•10 years ago
|
||
Sure, I'll take a look.
Assignee: nobody → giles
Flags: needinfo?(giles)
Assignee | ||
Comment 7•10 years ago
|
||
The crash reproduces on mac as well, as would be expected for an MP4Reader problem.
Assignee | ||
Comment 8•10 years ago
|
||
We assert because of a missing bounds check in the seek. I added an extra assert there. But we run off the end of the data because MPEG4Extractor loops forever looking for the 'trak' box. Or possibly in the origin it expects the io routines to assert and java to catch that as an exception. In any case, returning an error results in rejecting the file instead.
Attachment #8522642 -
Flags: review?(ajones)
Comment 9•10 years ago
|
||
Comment on attachment 8522642 [details] [diff] [review] Don't loop forever looking for tracks Review of attachment 8522642 [details] [diff] [review]: ----------------------------------------------------------------- I'm r+'ing this on the basis that I don't care what the if condition is provided that to doesn't exit when it sees a sidx. I'll leave it to you to get that right and I don't need to see another revision of this patch. ::: media/libstagefright/frameworks/av/media/libstagefright/MPEG4Extractor.cpp @@ +464,4 @@ > status_t err; > while (!mFirstTrack) { > err = parseChunk(&offset, 0); > + if (err) { parseChunk() generates an UNKNOWN_ERROR when it sees a sidx box but actually hasn't got an error. This will need to be something like: if (err && err != UNKNOWN_ERROR) Otherwise it won't skip over sidx boxes properly.
Attachment #8522642 -
Flags: review?(ajones) → review+
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
No, the whole thing causes some of the gtests to fail. I've been tracing through them but haven't gotten to the reason yet. I think the mock might be creating streams with no tracks?
Assignee | ||
Comment 12•10 years ago
|
||
Merge changes, carrying forward r=ajones. Sorry, I missed the cast in Anthony's patch. UNKNOWN_ERROR is unsigned but status_t is signed, so the comparison was _always_ true. I'd checked that it was skipping the 'sidx' atom but didn't notice we were skipping all errors. This prevented initialization from completing with the gtest MockMediaResource.
Attachment #8522642 -
Attachment is obsolete: true
Attachment #8523638 -
Attachment is obsolete: true
Attachment #8524189 -
Flags: review+
Assignee | ||
Comment 13•10 years ago
|
||
Pushed to try https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=12b91f14001e
Assignee | ||
Comment 14•10 years ago
|
||
Hmm, build failures in AnnexB.cpp. Something with unified build?
Assignee | ||
Comment 15•10 years ago
|
||
Nope, breakage from bug 1036849. https://hg.mozilla.org/integration/mozilla-inbound/rev/c7a24854801e
Backed out for crashtest failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/6b0a44c6fcc6 https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4006035&repo=mozilla-inbound
Flags: needinfo?(giles)
Assignee | ||
Comment 17•10 years ago
|
||
Looks like a mac-specific crash on dom/media/test/crashtests/0-timescale.html from the treeherder report. I cannot reproduce locally. Could be it's dependent on the replace_malloc changes, which I can't test because of bug 1101070. Will try again when that's fixed.
Flags: needinfo?(giles)
Comment 18•10 years ago
|
||
The patch may need to do "break" instead of "return err" because otherwise we don't set mInitCheck. I don't know exactly how this relates to the crashtest failure though.
Assignee | ||
Comment 19•10 years ago
|
||
That's correct. the early return was causing subsequent calls to fail at the assert on line 490. Using break instead resolves the crashtest issue. Carrying forward r=ajones.
Attachment #8524189 -
Attachment is obsolete: true
Attachment #8525596 -
Flags: review+
Assignee | ||
Comment 20•10 years ago
|
||
Trying again. https://hg.mozilla.org/integration/mozilla-inbound/rev/d24d7dfddb48
Reporter | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d24d7dfddb48
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 22•10 years ago
|
||
Backported fix to Aurora. Approval Request Comment [Feature/regressing bug #]: 1043696, 1057879. [User impact if declined]: Invalid files can crash the browser. [Describe test coverage new/current, TBPL]: Landed on m-c. [Risks and why]: We propagate more errors than we used to, so it's possible we'll reject more than just trackless files, but it is a small change. Risk is low. [String/UUID change made/needed]: none.
Attachment #8527090 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 23•10 years ago
|
||
Backported the version of the fix which passes tests to Aurora. Sorry for the confusion; I pulled from a stale local branch. Approval Request Comment [Feature/regressing bug #]: 1043696, 1057879. [User impact if declined]: Invalid files can crash the browser. [Describe test coverage new/current, TBPL]: Landed on m-c. [Risks and why]: We propagate more errors than we used to, so it's possible we'll reject more than just trackless files, but it is a small change. Risk is low. [String/UUID change made/needed]: none.
Attachment #8527090 -
Attachment is obsolete: true
Attachment #8527090 -
Flags: approval-mozilla-aurora?
Attachment #8527114 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 24•9 years ago
|
||
Greenish try push for the Aurora backport. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=865b8d7e4f7c
Updated•9 years ago
|
Attachment #8527114 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 25•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4e51a5678a9b
Updated•9 years ago
|
status-firefox35:
--- → fixed
status-firefox36:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•