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)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: cbook, Assigned: rillian)

References

()

Details

(Keywords: assertion)

Attachments

(3 files, 4 obsolete files)

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
Hi Chris, padenot told me that maybe your area (or you might know whats going on here)
Flags: needinfo?(cpearce)
We've got a recent orange in the WebRTC tests with the same assertion, in case that's relevant.
See Also: → 1094969
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)
(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
Ralph - do you have time to look into this?
Flags: needinfo?(ajones) → needinfo?(giles)
Sure, I'll take a look.
Assignee: nobody → giles
Flags: needinfo?(giles)
The crash reproduces on mac as well, as would be expected for an MP4Reader problem.
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 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+
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?
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+
Hmm, build failures in AnnexB.cpp. Something with unified build?
Blocks: 1043696
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)
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.
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+
https://hg.mozilla.org/mozilla-central/rev/d24d7dfddb48
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Attached patch Aurora backport (obsolete) — Splinter Review
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?
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?
Greenish try push for the Aurora backport. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=865b8d7e4f7c
Attachment #8527114 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.