884200 - crash in nsXPCWrappedJS::CallMethod because the directory service is being used from NSS initialization off the main thread

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Scoobidiver (away)]

It has been hit by one user on Android-x86.
Based on the stack trace, it's likely a regression from bug 770840.

Signature 	nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) More Reports Search
UUID	6f72b2a0-ac35-440d-aafb-2a6b42130617
Date Processed	2013-06-17 21:45:16
Uptime	25
Install Age	18.5 minutes since version was first installed.
Install Time	2013-06-17 21:26:32
Product	FennecAndroid
Version	24.0a1
Build ID	20130617031112
Release Channel	nightly
OS	Android
OS Version	0.0.0 Linux 3.0.34-00001-g77f517c2 #1 SMP PREEMPT Tue May 21 19:44:56 CST 2013 i686 asus/WW_epad/ME371MG:4.1.2/JZO54K/WW_epad-V3.2.2-20130521:user/release-keys
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 39 stepping 2
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'Imagination Technologies -- PowerVR SGX 540 -- OpenGL ES 2.0 build 1.9@2188537 -- Model: ME371MG, Product: WW_epad, Manufacturer: asus, Hardware: mfld_gi'
GL Layers! EGL? EGL+ GL Context? GL Context+ GL Layers+ 
nothumb Build
asus ME371MG
asus/WW_epad/ME371MG:4.1.2/JZO54K/WW_epad-V3.2.2-20130521:user/release-keys
Processor Notes 	sp-processor02_phx1_mozilla_com_22938:2012; non-integer value of "SecondsSinceLastCrash"; MDSW emitted too many frames, triggering truncation; exploitability tool: ERROR: unable to analyze dump
EMCheckCompatibility	True
Adapter Vendor ID	Imagination Technologies
Adapter Device ID	PowerVR SGX 540
Device	asus ME371MG
Android API Version	16 (REL)
Android CPU ABI	x86

Frame 	Module 	Signature 	Source
0 	libxul.so 	nsXPCWrappedJS::CallMethod 	js/xpconnect/src/XPCWrappedJS.cpp:571
1 	libxul.so 	libxul.so@0xd20f3c 	
2 	libxul.so 	xptiInterfaceEntry::GetMethodInfo 	xpcom/reflect/xptinfo/src/xptiInterfaceInfo.cpp:185
3 	libxul.so 	nsXPCWrappedJS::CallMethod 	js/xpconnect/src/XPCWrappedJS.cpp:568
4 	libxul.so 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_gcc_x86_unix.cpp:60
5 	libxul.so 	nsACString_internal::Equals const 	xpcom/string/src/nsTSubstring.cpp:612
6 	libnss3.so 	PR_Unlock 	nsprpub/pr/src/pthreads/ptsynch.c:205
7 	libxul.so 	SafeMutex::AssertNotCurrentThreadOwns const 	xpcom/components/nsComponentManager.h:110
8 	libxul.so 	NS_TableDrivenQI 	obj-firefox/xpcom/build/nsISupportsImpl.cpp:16
9 	libxul.so 	ReleaseData 	xpcom/string/src/nsSubstring.cpp:84
10 	libc.so 	libc.so@0xeb99 	
11 	libxul.so 	hashKey 	xpcom/ds/nsHashtable.cpp:57
12 	libxul.so 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/unix/xptcstubs_gcc_x86_unix.cpp:16
13 	libxul.so 	FindProviderFile 	xpcom/io/nsDirectoryService.cpp:362
14 	libxul.so 	nsHashtable::Get 	xpcom/ds/nsHashtable.cpp:205
15 	libxul.so 	nsSupportsHashtable::Get 	xpcom/ds/nsHashtable.cpp:787
16 	libxul.so 	FindProviderFile 	xpcom/io/nsDirectoryService.cpp:330
17 	libxul.so 	nsDirectoryService::Get 	xpcom/io/nsDirectoryService.cpp:394
18 	libxul.so 	CallGetService 	obj-firefox/xpcom/build/nsComponentManagerUtils.cpp:56
19 	libnss3.so 	PR_Lock 	nsprpub/pr/src/pthreads/ptsynch.c:183
...

More reports at:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=nsXPCWrappedJS%3A%3ACallMethod%28unsigned+short%2C+XPTMethodDescriptor+const*%2C+nsXPTCMiniVariant*%29

Comment 1

[Benjamin Smedberg]

The stackwalk on android x86 is completely busted for now, so don't trust it. This has nothing to do with xptiInterfaceEntry but I'm pretty sure that the bug is this:

* some necko thread

mozilla::net::HttpCacheQuery::OnCacheEntryAvailable
-> mozilla::net::HttpCacheQuery::CheckCache
-> mozilla::net::HttpCacheQuery::ResponseWouldVary const
-> mozilla::net::::Hash
-> component manager
-> nsCryptoHashConstructor
-> nsNSSComponentConstructor
-> nsNSSComponent::Initialize
-> http://hg.mozilla.org/mozilla-central/annotate/834c8941ae24/security/manager/ssl/src/nsNSSComponent.cpp#l1634
-> nsDirectoryService::Get *this is a main-thread-only API*
-> calls into a JS-implemented directory service provider off the main thread

The real problem here is that we continue to initialize NSS on arbitrary threads. It should be an invariant of our system that NSS can only be initialized on the main thread.

Comment 2

[Jason Duell]

> The real problem here is that we continue to initialize NSS on arbitrary threads. 
> It should be an invariant of our system that NSS can only be initialized on the 
> main thread.

Can we arrange to call NSS init early on the main thread to effectively guarantee that?

Comment 3

[Honza Bambas (:mayhemer)]

Just remove this condition: http://hg.mozilla.org/mozilla-central/annotate/7ba8c86f1a56/netwerk/protocol/http/nsHttpHandler.cpp#l1602

It will make PSM be init before any http channel lived.

Comment 4

[Jason Duell]

Attachment: v1: init NSS in nsHttpProtocolHandler::Init

It seemed more logical to init in the HTTP protocol handler's constructor than to check every time in :newProxiedChannel.  This does mean that it will get called a littler earlier though--my gbd shows the handler gets constructed from a call to register a ChromeWorker, which asks it for the AppVersion, not a new channel.

If that's too early I can write a patch that does the check in :newProxiedChannel.

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 766002 [details] [diff] [review]
v1: init NSS in nsHttpProtocolHandler::Init

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

Why do you need it that soon (in the ctor)?  To fix this bug you just need to instantiate psm for any created channel.. Your patch could influence startup unnecessarily.

Comment 6

[Jason Duell]

Attachment: v2

ok then :)

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 766144 [details] [diff] [review]
v2

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

yep :) thanks. r=honzab

Comment 8

[Jason Duell]

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

Comment 9

[Phil Ringnalda (:philor)]

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