Closed Bug 1367508 Opened 3 years ago Closed 3 years ago

Crash in alloc::oom::default_oom_handler

Categories

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

53 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: philipp, Assigned: ayang)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.
Flags: needinfo?(giles)
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?
Assignee: nobody → ayang
Flags: needinfo?(giles) → needinfo?(ayang)
(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.
Flags: needinfo?(ayang)
(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 on attachment 8873271 [details]
Bug 1367508 - Update rust parser to limit table size in case of OOM.

https://reviewboard.mozilla.org/r/144728/#review148624
Attachment #8873271 - Flags: review?(kinetik) → review+
(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!
Pushed by ayang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4229107601dd
Update rust parser to limit table size in case of OOM. r=kinetik
https://hg.mozilla.org/mozilla-central/rev/4229107601dd
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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)?
Flags: needinfo?(ayang)
(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.
Flags: needinfo?(ayang)
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?
Flags: needinfo?(ayang)
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
Flags: needinfo?(ayang) → needinfo?(bchien)
As Alfredo's comment 13, forward to Peter and Milan.
Flags: needinfo?(milan)
Flags: needinfo?(howareyou322)
Flags: needinfo?(bchien)
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.
Jeff, comment 16.
Flags: needinfo?(milan) → needinfo?(jmuizelaar)
Yes it looks like some of those functions needed to be added to the signature prefixes.
Flags: needinfo?(jmuizelaar)
I filed bug 1373272 about this.
Depends on: 1373272
Duplicate of this bug: 1373390
You need to log in before you can comment on or make changes to this bug.