1367508 - Crash in alloc::oom::default_oom_handler

Status: RESOLVED FIXED

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-b25bc0c2-1789-4d5b-8b87-25a860170524.
=============================================================
Crashing Thread (48)
Frame 	Module 	Signature 	Source
0 	xul.dll 	alloc::oom::default_oom_handler 	C:/bot/slave/stable-dist-rustc-win-msvc-32/build/src/liballoc/oom.rs:20
1 	xul.dll 	alloc::oom::oom 	C:/bot/slave/stable-dist-rustc-win-msvc-32/build/src/liballoc/oom.rs:31
2 	xul.dll 	alloc::raw_vec::RawVec<mp4parse::SampleToChunk>::double<mp4parse::SampleToChunk> 	C:/bot/slave/stable-dist-rustc-win-msvc-32/build/src/liballoc/raw_vec.rs:226
3 	xul.dll 	mp4parse::read_trak<mp4parse::BMFFBox<mp4parse_capi::mp4parse_io>> 	media/libstagefright/binding/mp4parse/src/lib.rs:732
4 	xul.dll 	mp4parse_capi::mp4parse_read 	media/libstagefright/binding/mp4parse_capi/src/lib.rs:298
5 	xul.dll 	mp4_demuxer::MP4MetadataRust::MP4MetadataRust(mp4_demuxer::Stream*) 	media/libstagefright/binding/MP4Metadata.cpp:658
6 	xul.dll 	mp4_demuxer::MP4Metadata::MP4Metadata(mp4_demuxer::Stream*) 	media/libstagefright/binding/MP4Metadata.cpp:159
7 	xul.dll 	mozilla::MP4Demuxer::Init() 	dom/media/fmp4/MP4Demuxer.cpp:141
8 	xul.dll 	mozilla::MediaFormatReader::AsyncReadMetadata() 	dom/media/MediaFormatReader.cpp:644
9 	xul.dll 	mozilla::detail::ProxyRunnable<mozilla::MozPromise<RefPtr<mozilla::MetadataHolder>, mozilla::MediaResult, 1>, RefPtr<mozilla::MozPromise<RefPtr<mozilla::MetadataHolder>, mozilla::MediaResult, 1> > ( mozilla::MediaDecoderReader::*)(void), mozilla::MediaDecoderReader>::Run() 	obj-firefox/dist/include/mozilla/MozPromise.h:1204
10 	xul.dll 	mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() 	obj-firefox/dist/include/mozilla/TaskDispatcher.h:193
11 	xul.dll 	mozilla::TaskQueue::Runner::Run() 	xpcom/threads/TaskQueue.cpp:232
12 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp:225
13 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1240
14 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:390
15 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:338
16 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:231
17 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:211
18 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:490
19 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
20 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
21 	ucrtbase.dll 	_o__CIpow 	
22 	kernel32.dll 	BaseThreadInitThunk 	
23 	ntdll.dll 	__RtlUserThreadStart 	
24 	ntdll.dll 	_RtlUserThreadStart

this crash signature on windows and android is popping up on firefox 53 and subsequent builds.

Comment 1

[Ralph Giles (:rillian)]

Alfredo, do you might looking at this? Could be another place we need to guard against large files. Is there a reasonable limit we could put on sample_count in read_stts for example?

Comment 2

[Alfredo Yang (:alfredo)]

(In reply to Ralph Giles (:rillian) | needinfo me from comment #1)
> Alfredo, do you might looking at this? Could be another place we need to
> guard against large files. Is there a reasonable limit we could put on
> sample_count in read_stts for example?

hmmm... In the worst case, one sample per chunk. Assume it is 30 fps video, so the table length of one day long video should be:

30 * 60 * 60 * 24 = 2592000

In read_stsc(), there are three u32 members in SampleToChunk. So the total memory consumption of this table is 31104000 bytes.
How about limiting it to one week long? So that'd be about 31MB * 7 = 217 MB per table.

Comment 3

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

(In reply to Alfredo Yang (:alfredo) from comment #2)
> How about limiting it to one week long? So that'd be about 31MB * 7 = 217 MB
> per table.

It seems reasonable not to support slow cinema via progressive download. https://en.wikipedia.org/wiki/List_of_longest_films

Comment 4

[Alfredo Yang (:alfredo)]

https://github.com/mozilla/mp4parse-rust/pull/97

Comment 5

[Alfredo Yang (:alfredo)]

Attachment: Bug 1367508 - Update rust parser to limit table size in case of OOM.

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

Comment 6

[Matthew Gregan [:kinetik]]

Comment on attachment 8873271 [details]
Bug 1367508 - Update rust parser to limit table size in case of OOM.

https://reviewboard.mozilla.org/r/144728/#review148624

Comment 7

[Alfredo Yang (:alfredo)]

(In reply to Matthew Gregan [:kinetik] from comment #6)
> Comment on attachment 8873271 [details]
> Bug 1367508 - Update rust parser to limit table size in case of OOM.
> 
> https://reviewboard.mozilla.org/r/144728/#review148624

Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4229107601dd
Update rust parser to limit table size in case of OOM. r=kinetik

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4229107601dd

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Can this fix ride the 55 train or is it something that should stay on the radar for 54 dot release consideration (if there ends up being one)?

Comment 11

[Alfredo Yang (:alfredo)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> Can this fix ride the 55 train or is it something that should stay on the
> radar for 54 dot release consideration (if there ends up being one)?

It happens on extra long/malformat video so I think it's good on 55 train.

Comment 12

[Marcia Knous [:marcia]]

This signature is still present in 56 - http://bit.ly/2s0HRY3 as well as some crashes in 55 (I see one in 20170610030207). Alfred can you take a look?

Comment 13

[Alfredo Yang (:alfredo)]

There are two OOM in JS [1], [2]. Others are OOM in webrender. None of them has the same symptom as before.
Maybe we should have graphic team to check it?

[1] https://crash-stats.mozilla.com/report/index/305c53fc-89bd-4a88-8684-07ec10170613
[2] https://crash-stats.mozilla.com/report/index/e28a2c38-88db-4662-acbe-cc83b0170614

Comment 14

[Bobby Chien]

As Alfredo's comment 13, forward to Peter and Milan.

Comment 15

[Peter Chang[:pchang]]

For OOM in webrender, it happened within GPU process. So we can search crash reports with process type as GPU in [1].

I listed the crash reports within 7 days.

1 	56.0a1 	21 	72.41 %
2 	55.0a1 	8 	27.59 %

[1]https://crash-stats.mozilla.com/signature/?product=Firefox&process_type=~gpu&signature=alloc%3A%3Aoom%3A%3Adefault_oom_handler&date=%3E%3D2017-06-08T08%3A18%3A00.000Z&date=%3C2017-06-15T08%3A18%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports

Comment 16

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Some reports are not like oom problem.

e.g.
https://crash-stats.mozilla.com/report/index/e98bf5b1-2031-4a28-a594-8538f0170613

This crash is triggered by the "unreachable!()" call.
https://hg.mozilla.org/mozilla-central/annotate/5293e5f89358/gfx/webrender/src/device.rs#l1679

Does rust have only one panic hook? Do we set the oom handler as the global panic hook? If yes, the non-oom crashes will also be aggregated here.

Comment 17

[Milan Sreckovic [:milan] (needinfo for best results)]

Jeff, comment 16.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Yes it looks like some of those functions needed to be added to the signature prefixes.

Comment 19

[Jeff Muizelaar [:jrmuizel]]

I filed bug 1373272 about this.