573078 - [Mac] OOM in nsHtml5StreamParser::WriteStreamBytes [@ mozalloc_abort(char const*) ]

Status: RESOLVED FIXED

Description

[Marcia Knous [:marcia - use ni]]

Seen while reviewing trunk crash stats. Now the #14 overall top crash on trunk and #1 top crash on Mac.

http://tinyurl.com/25q7vnm links to all current crashes. It appears the crashes started on 6/11 and then spiked on 6/17.

Not many URLs to go on as many of the crashes happened right after startup. I contacted on mozilla person who crashed and he said he had no idea how it happened.

Comment 1

[Marcia Knous [:marcia - use ni]]

Frame  	Module  	Signature [Expand]  	Source
0 	libmozalloc.dylib 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:64
1 	libmozalloc.dylib 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:54
2 	libmozalloc.dylib 	libmozalloc.dylib@0xcc0 	
3 	XUL 	XUL@0x62bad6 	
4 	XUL 	nsHtml5StreamParser::WriteStreamBytes 	parser/html/nsHtml5StreamParser.cpp:513
5 	XUL 	nsHtml5StreamParser::DoDataAvailable 	parser/html/nsHtml5StreamParser.cpp:665
6 	XUL 	nsHtml5DataAvailable::Run 	parser/html/nsHtml5StreamParser.cpp:705
7 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:547
8 	XUL 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:250
9 	XUL 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:263
10 	libnspr4.dylib 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c:228
11 	libSystem.B.dylib 	_pthread_start 	
12 	libSystem.B.dylib 	thread_start

Comment 2

[Marcia Knous [:marcia - use ni]]

Approaching the 500 crash mark, which is a high for a Mac nightly. http://tinyurl.com/2fasce3 links to the crashes, which are mostly startup crashes.

Comments mention:

trying to start minefield while firefox is running
I'm just trying to start up.

https://crash-stats.mozilla.com/report/index/280ac94b-a672-4af2-b526-6ce992100622 is interesting in that a line in that stack indicates: _ZL20ProfileMissingDialogP19nsINativeAppSupport

Comment 3

[Steven Michaud [:smichaud] (Retired)]

These crashes don't show up when you filter on the "1.9.3 branch" -- as I've been used to doing for the last few months.  So I completely missed them.

Wierd!

Comment 4

[Steven Michaud [:smichaud] (Retired)]

They appear to all be null dereferences at:

http://hg.mozilla.org/mozilla-central/annotate/431eab8cf6ab/memory/mozalloc/mozalloc_abort.cpp#l64

Comment 5

[Steven Michaud [:smichaud] (Retired)]

I don't know how to categorize this bug report.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This stack from comment 1 signifies OOM in the HTML5 parser.  If that stack starts moving up the topcrash leaderboard, the allocating code here needs to switch to explicitly fallible allocation.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Actually, these *are* Mac-specific.

I've seen reports for OS X 10.5.8 and various versions of 10.6.X.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

The number of these crashes over the last two weeks (460) is truly dramatic.  The next legitimate entry in the Mac topcrasher list (js_UnboxDouble(long)) has only 8 crashes!  (The crashes at nsCrasher::Crash(short) are in the "crashme" testing extension.)

http://crash-stats.mozilla.com/query/query?product=Firefox&version=Firefox%3A3.7a6pre&platform=mac&range_value=2&range_unit=weeks&date=06%2F23%2F2010+12%3A52%3A43&query_search=signature&query_type=exact&query=&build_id=&process_type=any&hang_type=any&do_query=1

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Fortunately A5 (based on the 2010-06-10 nightly) doesn't have any of these crashes.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

From a random sampling of the reports in comment 8, it appears to me that the majority are

0  	libmozalloc.dylib  	mozalloc_abort  	 memory/mozalloc/mozalloc_abort.cpp:64
1 	XUL 	NS_DebugBreak_P 	xpcom/base/nsDebugImpl.cpp:379
2 	XUL 	XUL@0xc8185c 	
3 	XUL 	base::ThreadLocalPlatform::SetValueInSlot 	ipc/chromium/src/base/logging.h:61
4 	XUL 	XUL@0xc83000 	
5 	XUL 	NS_InitXPCOM3_P 	ipc/chromium/src/base/message_loop.h:434
6 	XUL 	ScopedXPCOMStartup::Initialize 	toolkit/xre/nsAppRunner.cpp:1145
7 	XUL 	ProfileMissingDialog 	toolkit/xre/nsAppRunner.cpp:1901
8 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3264
9 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
10 	firefox-bin 	firefox-bin@0xbf5 	
11 		@0x1

I did see some crashes in the HTML5 parser that looked the same as comment 1, so I suggest we morph this bug into the OOM tracker and spin off a new bug for the startup problem.  (The OOM shouldn't be mac-specific, but one never knows ...)

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Spun off initialization problem into bug 574139.

Comment 12

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

(In reply to comment #1)
> Frame      Module      Signature [Expand]      Source
> 0     libmozalloc.dylib     mozalloc_abort    
> memory/mozalloc/mozalloc_abort.cpp:64
> 1     libmozalloc.dylib     mozalloc_handle_oom    
> memory/mozalloc/mozalloc_oom.cpp:54
> 2     libmozalloc.dylib     libmozalloc.dylib@0xcc0     
> 3     XUL     XUL@0x62bad6

I'm guessing stack frame #3 has to be the constructor for nsHtml5UTF16Buffer. I wonder why there's no symbol.

> 4     XUL     nsHtml5StreamParser::WriteStreamBytes    
> parser/html/nsHtml5StreamParser.cpp:513

Assuming that stack frame #3 is the constructor for nsHtml5UTF16Buffer, this crash is a case where the HTML5 parser allocates 1 KB at a time. Does the infallible malloc already have a mechanism for freeing stuff (killing tabs?) when it runs low on memory? If it does, is 1 KB too much to allocate at a time for the mechanism to work?

If I made the HTML5 parser return NS_ERROR_OUT_OF_MEMORY to the event loop of the parser thread, what would happen? If the next buffer were pushed to the parser nonetheless and it happened to succeed, a piece of the page would be missing, which could change the executability properties of pieces of the page.

Comment 13

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

(In reply to comment #12)
> Does the
> infallible malloc already have a mechanism for freeing stuff (killing tabs?)
> when it runs low on memory?

Apparently not:
http://mxr-test.konigsberg.mozilla.org/mozilla-central/source/memory/mozalloc/mozalloc_oom.cpp#49
:-(

Comment 14

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

What to do here really depends on whether the infallible malloc becomes able to stop active parsers or (failing that) zap existing tabs/windows to free memory.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We're not going to have tabs in their own process(es) until FF 5.  That gives us more OOM-handling options.  Until then, this allocation site should be made fallible and (probably) bail on OOM.

(In reply to comment #12)
> If I made the HTML5 parser return NS_ERROR_OUT_OF_MEMORY to the event loop of
> the parser thread, what would happen? If the next buffer were pushed to the
> parser nonetheless and it happened to succeed, a piece of the page would be
> missing, which could change the executability properties of pieces of the page.

I can't answer that question, but it'd be easy enough to simulate an OOM here and find out.

Comment 16

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

(In reply to comment #15)
> We're not going to have tabs in their own process(es) until FF 5.  That gives
> us more OOM-handling options.  Until then, this allocation site should be made
> fallible and (probably) bail on OOM.

OK. :-(

> (In reply to comment #12)
> > If I made the HTML5 parser return NS_ERROR_OUT_OF_MEMORY to the event loop of
> > the parser thread, what would happen? If the next buffer were pushed to the
> > parser nonetheless and it happened to succeed, a piece of the page would be
> > missing, which could change the executability properties of pieces of the page.
> 
> I can't answer that question, but it'd be easy enough to simulate an OOM here
> and find out.

It seems to me that if this allocation fails, the parser needs to be flagged broken and subsequent buffer should be ignored, too, for security reasons.

Comment 17

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

Safe (as in not allowing arbitrary code execution) OOM crashes in the parser are now lower priority than Web-facing correctness fixes.

Comment 18

[Henri Sivonen (:hsivonen) (not doing reviews or reading bugmail until 2017-07-31)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/daef044609ce

Comment 19

[Ed Morley (Away 27th-2nd) [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/daef044609ce