Closed Bug 1171629 Opened 5 years ago Closed 5 years ago

crash in OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xrealloc | nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity(unsigned int, unsigned int) | nsTArray_Impl<mp4_demuxer:...

Categories

(Core :: Audio/Video, defect, critical)

39 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 + fixed
firefox40 + fixed
firefox41 - fixed

People

(Reporter: dmajor, Assigned: jya)

Details

(Keywords: crash)

Crash Data

Attachments

(3 files)

This bug was filed from the Socorro interface and is 
report bp-9e8c55d0-e546-4a00-a2be-eeea52150604.
=============================================================

mozglue!mozalloc_abort
mozglue!mozalloc_handle_oom
mozglue!moz_xrealloc
xul!nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyWithMemutils>::EnsureCapacity
xul!nsTArray_Impl<mp4_demuxer::Sample,nsTArrayInfallibleAllocator>::AppendElement<mp4_demuxer::Sample &>
xul!mp4_demuxer::Index::Index
xul!mp4_demuxer::MP4Demuxer::Init
xul!mozilla::InvokeAndRetry<mozilla::MP4Reader,bool>
xul!mozilla::MP4Reader::ReadMetadata
xul!mozilla::MediaDecoderReader::CallReadMetadata
xul!mozilla::detail::MethodCallWithNoArgs<mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>,enum mozilla::ReadMetadataFailureReason,1>,mozilla::MediaDecoderReader>::Invoke
xul!mozilla::detail::ProxyRunnable<mozilla::MediaPromise<nsRefPtr<mozilla::MetadataHolder>,enum mozilla::ReadMetadataFailureReason,1> >::Run
xul!mozilla::MediaTaskQueue::Runner::Run
xul!nsThreadPool::Run
Here is |aIndex| in Index::Index. Is is legitimate to have an array so large? If so, can we make the allocations fallible and/or segmented?

class stagefright::Vector<stagefright::MediaSource::Indice> * 0x2e8df65c
   +0x000 __VFN_table : 0x111493f8 
   +0x004 mStorage         : 0x7a300010 Void
   +0x008 mCount           : 0x6a0f2
   +0x00c mFlags           : 0
   +0x010 mItemSize        : 0x28
Flags: needinfo?(ajones)
Aside: The loop in Index::Index should probably allocate space up-front, to save on resize thrashing. (It wouldn't solve this OOM, though)
Oops. I meant to type the Aside and do the tracking flags as different comments!

Flagging for 39 beacause it's one of the top crash scores on 39b2. This signature is also seen on 40, but not 41. Unclear whether 41 is unaffected or whether it's just a result of the nightly population.
[Tracking Requested - why for this release]:
Tracking 39/40 because they are affected. Awaiting more information for tracking 41.
Jean-Yves - I think this is coming up due to dormant mode not working so well. We can probably improve it a little by nulling out the index from libstagefright and also making the copy fallable. I can't recall for sure whether there is a temporary copy we can avoid.
Flags: needinfo?(jyavenard)
...and I forgot to say - can you look into this one?
A plain MP4 stores an array of samples in the moov box.

The OOM above is for a 434418 samples, which is about a 2 hours movie with a video at 60Hz.

As we are now using the new Index stuff to access plain MP4, it does a copy of the stagefright array of samples.

Will make the failure fallible and allocated in one go.
Flags: needinfo?(jyavenard)
Assignee: nobody → jyavenard
Make memory allocation fallible. Also set array capacity in one go to reduce memory fragmentation
Attachment #8617206 - Flags: review?(ajones)
Patch for aurora
Comment on attachment 8617206 [details] [diff] [review]
Use fallible array to store MP4 samples index

Review of attachment 8617206 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8617206 - Flags: review?(ajones) → review+
Flags: needinfo?(ajones)
https://hg.mozilla.org/mozilla-central/rev/e86a8ea9ce53
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Jean-Yves, can you request uplift if you think this should move up to 39 or 40? Thanks.
Flags: needinfo?(jyavenard)
Crash Signature: , ...] → , ...] [@ OOM | large | mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xrealloc | nsTArray_base<T>::EnsureCapacity(unsigned int, unsigned int) | nsTArray_Impl<T>::AppendElement<T>(mp4_demuxer::Sample&) ] [@ OOM | large | moz…
Comment on attachment 8617208 [details] [diff] [review]
Use fallible array to store MP4 samples index

Approval Request Comment
[Feature/regressing bug #]: 1098126
[User impact if declined]: Crashes (OOM)
[Describe test coverage new/current, TreeHerder]: In m-c, manual testing
[Risks and why]: Low, we made an infallible operation, fallibles.
[String/UUID change made/needed]: None
Flags: needinfo?(jyavenard)
Attachment #8617208 - Flags: approval-mozilla-aurora?
Comment on attachment 8617227 [details] [diff] [review]
Use fallible array to store MP4 samples index

Approval Request Comment
[Feature/regressing bug #]: 1098126
[User impact if declined]: Crashes (OOM)
[Describe test coverage new/current, TreeHerder]: In m-c, manual testing
[Risks and why]: Low, we made an infallible operation, fallibles.
[String/UUID change made/needed]: None
Attachment #8617227 - Flags: approval-mozilla-beta?
Comment on attachment 8617208 [details] [diff] [review]
Use fallible array to store MP4 samples index

Approved for uplift to aurora.
Attachment #8617208 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8617227 [details] [diff] [review]
Use fallible array to store MP4 samples index

Approved for uplift to beta.
Attachment #8617227 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Since this is an old bug that has been fixed for a few weeks, it probably doesn't make sense to track it. If the bug is reopened, please re-nominate for tracking for FF41.
Crash Signature: , unsigned int) | nsTArray_Impl<T>::AppendElement<T>(mp4_demuxer::Sample&) ] [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xrealloc | nsTArray_base<T>::EnsureCapacity | nsTArray_Impl<T>::AppendElement<T> ] → , unsigned int) | nsTArray_Impl<T>::AppendElement<T>(mp4_demuxer::Sample&) ] [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xrealloc | nsTArray_base<T>::EnsureCapacity | nsTArray_Impl<T>::AppendElement<T> ] [@ OOM | large | mozalloc_abort |…
You need to log in before you can comment on or make changes to this bug.