Closed
Bug 1171629
Opened 9 years ago
Closed 9 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)
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: away, Assigned: jya)
Details
(Keywords: crash)
Crash Data
Attachments
(3 files)
12.48 KB,
patch
|
ajones
:
review+
|
Details | Diff | Splinter Review |
11.65 KB,
patch
|
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
6.85 KB,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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)
status-firefox39:
--- → affected
tracking-firefox39:
--- → ?
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.
status-firefox40:
--- → affected
status-firefox41:
--- → ?
Comment 4•9 years ago
|
||
[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?
Assignee | ||
Comment 7•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → jyavenard
Assignee | ||
Comment 8•9 years ago
|
||
Make memory allocation fallible. Also set array capacity in one go to reduce memory fragmentation
Attachment #8617206 -
Flags: review?(ajones)
Assignee | ||
Comment 9•9 years ago
|
||
Patch for aurora
Assignee | ||
Comment 10•9 years ago
|
||
Patch for beta
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+
Updated•9 years ago
|
Flags: needinfo?(ajones)
https://hg.mozilla.org/mozilla-central/rev/e86a8ea9ce53
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 14•9 years ago
|
||
Jean-Yves, can you request uplift if you think this should move up to 39 or 40? Thanks.
Flags: needinfo?(jyavenard)
Updated•9 years ago
|
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…
Assignee | ||
Comment 15•9 years ago
|
||
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?
Assignee | ||
Comment 16•9 years ago
|
||
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 17•9 years ago
|
||
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 18•9 years ago
|
||
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.
Updated•8 years ago
|
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.
Description
•