874158 - crash in xpc::GetNativeForGlobal

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Scoobidiver (away)]

It's #16 browser crasher in 23.0a2 (many duplicates) and #221 in 24.0a1.

It first showed up in 23.0a1/20130420. The regression range might be (startup):
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=64d6d002e888&tochange=dd03d42b01b1
Based on the stack trace, it's likely a regression from bug 820170.

Signature 	xpc::GetNativeForGlobal(JSObject*) More Reports Search
UUID	aa755d21-dd48-4ef2-9e55-224aa2130519
Date Processed	2013-05-19 21:50:20
Uptime	17
Last Crash	1.2 minutes before submission
Install Age	1.7 minutes since version was first installed.
Install Time	2013-05-19 21:48:14
Product	Firefox
Version	24.0a1
Build ID	20130519031035
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 15 model 4 stepping 1
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
Processor Notes 	sp-processor04_phx1_mozilla_com_29616:2012; WARNING: raw_crash missing Add-ons
EMCheckCompatibility	False	
Total Virtual Memory	2147352576
Available Virtual Memory	1940844544
System Memory Use Percentage	66
Available Page File	2420068352
Available Physical Memory	819036160

Frame 	Module 	Signature 	Source
0 	xul.dll 	xpc::GetNativeForGlobal 	js/xpconnect/wrappers/WrapperFactory.cpp:712
1 	xul.dll 	nsDocument::Init 	content/base/src/nsDocument.cpp:1907
2 	xul.dll 	nsHTMLDocument::Init 	content/html/document/src/nsHTMLDocument.cpp:254
3 	xul.dll 	NS_NewHTMLDocument 	content/html/document/src/nsHTMLDocument.cpp:176
4 	xul.dll 	CreateHTMLDocument 	layout/build/nsLayoutModule.cpp:543
5 	xul.dll 	mozilla::GenericFactory::CreateInstance 	obj-firefox/xpcom/build/GenericFactory.cpp:16
6 	xul.dll 	nsComponentManagerImpl::CreateInstance 	xpcom/components/nsComponentManager.cpp:1008
7 	xul.dll 	nsCreateInstanceByCID::operator 	obj-firefox/xpcom/build/nsComponentManagerUtils.cpp:167
8 	xul.dll 	nsCOMPtr_base::assign_from_helper 	obj-firefox/xpcom/build/nsCOMPtr.cpp:110
9 	xul.dll 	nsContentDLF::CreateBlankDocument 	layout/build/nsContentDLF.cpp:303
10 	xul.dll 	nsDocShell::CreateAboutBlankContentViewer 	docshell/base/nsDocShell.cpp:6986
11 	xul.dll 	nsDocShell::CreateAboutBlankContentViewer 	docshell/base/nsDocShell.cpp:7023
12 	xul.dll 	nsWebShellWindow::Initialize 	xpfe/appshell/src/nsWebShellWindow.cpp:219
13 	xul.dll 	nsAppShellService::JustCreateTopWindow 	xpfe/appshell/src/nsAppShellService.cpp:588
14 	xul.dll 	nsAppShellService::CreateHiddenWindowHelper 	xpfe/appshell/src/nsAppShellService.cpp:128
15 	xul.dll 	nsAppShellService::CreateHiddenWindow 	xpfe/appshell/src/nsAppShellService.cpp:88
16 	xul.dll 	nsAppStartup::CreateHiddenWindow 	toolkit/components/startup/nsAppStartup.cpp:238
17 	xul.dll 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3806
18 	xul.dll 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3939
19 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:4151
20 	firefox.exe 	do_main 	browser/app/nsBrowserApp.cpp:272
21 	firefox.exe 	NS_internal_main 	browser/app/nsBrowserApp.cpp:632
22 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:105
23 	firefox.exe 	__tmainCRTStartup 	crtexe.c:552
24 	kernel32.dll 	BaseThreadInitThunk 	
25 	ntdll.dll 	__RtlUserThreadStart 	
26 	ntdll.dll 	_RtlUserThreadStart 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AGetNativeForGlobal%28JSObject*%29

Comment 1

[Gabor Krizsanits (INACTIVE)]

The crash happens because the creation of junk scope in some weird cases (too early?) fails, and instead of passing it to GetNativeForGlobal we should do a null check on it and return failure from nsDocument::Init to avoid the crash.

The real problem however, is that creating junk scope is now pretty much required to create a document, which means it should always succeed when we should be able to create a document. I guess it fails because xpconnect is not ready? Bobby what do you think? How could we make this more robust?

Comment 2

[Bobby Holley (:bholley)]

I think we need to figure out exactly why we're failing here. All of this stuff (getting XPConnect, getting the runtime, getting the safe JSContext, getting the junk scope) should lazily create what it needs to, so it's not clear to me what's going wrong. 

From the looks of it, it seems most likely that xpc_CreateSandboxObject is failing. I would have guessed that maybe CAPS wasn't set up yet, but if so I'm pretty sure the call to nsContentUtils::GetSystemPrincipal() in GetJunkScope would null deref, and we'd see that here. So it's hard to tell exactly what's failing.

Note that trevor is cleaning up some of this stuff in bug 873622 to make XPConnect initialization happen explicitly in nsLayoutStatics and making the XPConnect getter infallible.

Comment 3

[Gabor Krizsanits (INACTIVE)]

(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #2)
> Note that trevor is cleaning up some of this stuff in bug 873622 to make
> XPConnect initialization happen explicitly in nsLayoutStatics and making the
> XPConnect getter infallible.

Hopefully that will fix this... I cannot reproduce this at all, the best I can do is ensuring in XPCJSRuntime::newXPCJSRuntime that the junk scope is created.

Comment 4

[Tracy Walker [:tracy]]

Module correlations:
EXCEPTION_ACCESS_VIOLATION_READ (18)

89% (16/18) vs.  19% (313/1624) aswJsFlt.dll
89% (16/18) vs.  22% (362/1624) snxhk.dll
67% (12/18) vs.   1% (12/1624) auCOLPwd.dll
67% (12/18) vs.  21% (344/1624) oleacc.dll
67% (12/18) vs.  28% (448/1624) dhcpcsvc6.DLL
67% (12/18) vs.  33% (537/1624) dhcpcsvc.dll
100% (18/18) vs.  75% (1211/1624) lpk.dll
94% (17/18) vs.  72% (1172/1624) secur32.dll
22% (4/18) vs.   3% (52/1624) Datamngr.dll
22% (4/18) vs.   3% (53/1624) mgrldr.dll
22% (4/18) vs.   6% (95/1624) sprotector.dll
28% (5/18) vs.  12% (193/1624) MSCTFIME.IME
33% (6/18) vs.  19% (301/1624) MSCTF.dll
100% (18/18) vs.  87% (1419/1624) browsercomps.dll
100% (18/18) vs.  88% (1426/1624) firefox.exe
100% (18/18) vs.  88% (1428/1624) dbghelp.dll
22% (4/18) vs.  10% (165/1624) ashTask.dll
22% (4/18) vs.  10% (165/1624) aswCmnOS.dll
22% (4/18) vs.  10% (165/1624) aswAux.dll
22% (4/18) vs.  10% (165/1624) aswProperty.dll
22% (4/18) vs.  10% (165/1624) aswEngLdr.dll
22% (4/18) vs.  10% (165/1624) avastIP.dll
22% (4/18) vs.  10% (165/1624) aswCmnBS.dll
22% (4/18) vs.  10% (165/1624) aswCmnIS.dll
22% (4/18) vs.  10% (165/1624) ashBase.dll
22% (4/18) vs.  10% (165/1624) AavmRpch.dll
22% (4/18) vs.  10% (165/1624) Aavm4h.dll
100% (18/18) vs.  90% (1460/1624) mswsock.dll
33% (6/18) vs.  24% (388/1624) hnetcfg.dll
33% (6/18) vs.  24% (388/1624) wshtcpip.dll
22% (4/18) vs.  13% (218/1624) wtsapi32.dll
67% (12/18) vs.  58% (949/1624) WSHTCPIP.DLL
33% (6/18) vs.  25% (400/1624) comres.dll
33% (6/18) vs.  25% (403/1624) iphlpapi.dll
33% (6/18) vs.  25% (403/1624) ws2help.dll
22% (4/18) vs.  17% (271/1624) winsta.dll

Comment 5

[Scoobidiver (away)]

(In reply to Tracy Walker [:tracy] from comment #4)
> Module correlations:
One-day correlations are meaningless for startup crashes with many duplicates.

Comment 6

[Scoobidiver (away)]

It's #7 top browser crasher in 23.0b3 still with many duplicates meaning either it happens also in Safe Mode or the Safe Mode prompt for startup crashes doesn't show up.

Comment 7

[bhavana bajaj [:bajaj]]

Tracking for Fx23 given its a top-crash, Gabor given bug 873622 has no impact on crash volume are there any more speculative fixes that we could try here ?

Comment 8

[Robert Kaiser]

It looks to me like https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AGetNativeForGlobal%28JSObject*%29 stopped after 23.0b2, and https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AEnsureCompartmentPrivate(JSObject*) is #22 on 23.0b3 from what I'm seeing now.

Comment 9

[Gabor Krizsanits (INACTIVE)]

(In reply to bhavana bajaj [:bajaj] from comment #7)
> Tracking for Fx23 given its a top-crash, Gabor given bug 873622 has no
> impact on crash volume are there any more speculative fixes that we could
> try here ?

Yes. I can make the document creation fail instead of crashing. Junk scope creation should never fail, but apparently it does, so until it is fixed I will make sure that every consumer is prepared for that case... I wish I could figure out what fails exactly and fix that, but I have nothing.

Comment 10

[Scoobidiver (away)]

This startup crash is high at the beginning of each Beta (#11 in 23.0b4) and decreases slowly then so it has a good chance to be high in 23.0 Release.

Comment 11

[Gabor Krizsanits (INACTIVE)]

https://tbpl.mozilla.org/?tree=Try&rev=9ac59153f514

Comment 12

[Gabor Krizsanits (INACTIVE)]

Attachment: Handle if GetJunkScope returns nullptr

GetJunkScope should never fail, but that patch is not trivial as I don't see how does it actually fail sometimes. Probably it should not use a sandbox. I've added a GetJunkScopeGlobal that returns nsIGlobalObject since that is the function all the current consumers really need, and dealing with JSObject in DOM code is not something we prefer anyway. The patch is trivial, instead of crashing, we just fail the init functions. I wish I had something better...

Comment 13

[Bobby Holley (:bholley)]

Comment on attachment 774496 [details] [diff] [review]
Handle if GetJunkScope returns nullptr

Review of attachment 774496 [details] [diff] [review]:
-----------------------------------------------------------------

r=bholley

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +400,5 @@
> +GetJunkScopeGlobal()
> +{
> +    JSObject *junkScope = GetJunkScope();
> +    // GetJunkScope would ideally never fail, currently it is not yet the case
> +    // unfortunately...

Please add a reference to this bug in the comment.

Comment 14

[Gabor Krizsanits (INACTIVE)]

https://tbpl.mozilla.org/?tree=Try&rev=d06b1ccc422a

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

Comment 15

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 774496 [details] [diff] [review]
Handle if GetJunkScope returns nullptr

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 820170
User impact if declined: crash (about frequency see comments...)
Testing completed (on m-c, etc.): I could not reproduce the test, so I cannot create a test for it either.
Risk to taking this patch (and alternatives if risky): This patch is not risky at all. 
String or IDL/UUID changes made by this patch: none

So this patch only makes some init functions (document, xhr, DOMFileReader)  fail instead of crashing if the junk scope cannot be created for some reason. This scenario should not happen, but it does happen every now and then for some unknown reason (usually at startup).

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c0dfc5460b9d
https://hg.mozilla.org/releases/mozilla-beta/rev/3eeb8f19f8b4

Comment 18

[Ioana (away)]

https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AGetNativeForGlobal%28JSObject%2A%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&hang_type=any&date=2013-08-01+11%3A00%3A00&range_value=4

https://crash-stats.mozilla.com/report/list?signature=xpc%3A%3AEnsureCompartmentPrivate%28JSObject%2A%29&product=Firefox&query_type=contains&range_unit=weeks&process_type=any&hang_type=any&date=2013-08-01+11%3A00%3A00&range_value=4

There are five crashes on the first beta after the fix, but nothing else after that. No new crashes on Firefox 24 and 25 either.