1364828 - Crash in moz_abort | arena_run_split | arena_malloc_large | je_realloc | mozilla::Vector<T>::growStorageBy

Status: RESOLVED FIXED

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

Description

[Zbigniew Szczesny]

This bug was filed from the Socorro interface and is 
report bp-2ee8a067-688d-46a6-b404-dac180170513.
=============================================================

Comment 1

[Zbigniew Szczesny]

Constant crashes somehow linked to certain video elements on a webpage.

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Jean-Yves - is this an OOM by another name?

Comment 3

[Jean-Yves Avenard [:jya]]

That code is supposed to be fallible.. :gerald made it so...

The stack trace is a bit confusing, looks like it has inline the writer code.

:gerald what do you think?

Comment 4

[Gerald Squelart (he/him)]

Hmm, I'm not 100% sure now...

I assumed Vector::append was fallible because of the `MOST_MUST_USE bool` return value.

But looking at https://hg.mozilla.org/releases/mozilla-aurora/annotate/105e456d811b/mfbt/Vector.h#l680 , it says "Potentially fallible append operations" -- Does that mean that these operations can potentially fail when we ask for too much, or they can potentially fail depending on what type of objects are stored??

And trying to follow the code for Vector<uint8_t>, it seems to go to pod_alloc instead of maybe_pod_alloc, so we may in fact be doomed!

But I need more time to investigate...

Comment 5

[Gerald Squelart (he/him)]

Attachment: Bug 1364828 - Use nsTArray (with fallible append) in mp4_demuxer::ByteWriter -

mfbt's Vector is actually infallible by default, so it would crash in case of
OOM. I'm switching to nsTArray, and using fallible AppendElements in
ByteWriter.

Review commit: https://reviewboard.mozilla.org/r/146462/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146462/

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8875091 [details]
Bug 1364828 - Use nsTArray (with fallible append) in mp4_demuxer::ByteWriter -

https://reviewboard.mozilla.org/r/146462/#review150640

nsTArray has a great memory footprint than Vector :(
it allocates memory by a multiple of 2... and IIRC 8kB minimum

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4795dae51ea6
Use nsTArray (with fallible append) in mp4_demuxer::ByteWriter - r=jya

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4795dae51ea6

Comment 9

[Jean-Yves Avenard [:jya]]

Both 53, 54 and esr52 are affected as the code was there too... May not show in the crash report but it's definitely there.

Bug 1307945 was supposed to make things infallible, but wasn't quite so.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

There's a couple ESR52 reports in crash-stats, but doesn't seem high-volume enough to warrant backport consideration. Feel free to set the status back to affected and nominate it for approval if you feel otherwise, however :)