827272 - crash in nsSSLIOLayerHelpers::Cleanup

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[Scoobidiver (away)]

It's #31 top browser crasher in the crash-noisy 20.0a1, and #6 non-fixed top browser crasher.
It first showed up in 20.0a1/20121214. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=edd45de440ba&tochange=b11065872128
It's likely a regression from bug 769288.

Signature 	nsSSLIOLayerHelpers::Cleanup() More Reports Search
UUID	b912a0c5-237f-4a77-a502-5a0c52130107
Date Processed	2013-01-07 11:17:59
Uptime	0
Last Crash	32 seconds before submission
Install Age	8.5 hours since version was first installed.
Install Time	2013-01-07 17:47:21
Product	Firefox
Version	20.0a1
Build ID	20130106030902
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7600
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 42 stepping 7
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x10
EMCheckCompatibility	True
Total Virtual Memory	2147352576
Available Virtual Memory	1965981696
System Memory Use Percentage	25
Available Page File	5948133376
Available Physical Memory	2555514880

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsSSLIOLayerHelpers::Cleanup 	security/manager/ssl/src/nsNSSIOLayer.cpp:496
1 	xul.dll 	mozilla::psm::SharedSSLState::GlobalCleanup 	security/manager/ssl/src/SharedSSLState.cpp:190
2 	xul.dll 	nsNSSComponent::~nsNSSComponent 	security/manager/ssl/src/nsNSSComponent.cpp:403
3 	xul.dll 	nsNSSComponent::`vector deleting destructor' 	
4 	xul.dll 	nsNSSComponent::Release 	security/manager/ssl/src/nsNSSComponent.cpp:1969
5 	xul.dll 	nsNSSComponentConstructor 	security/manager/ssl/src/nsNSSModule.cpp:163
6 	xul.dll 	mozilla::GenericFactory::CreateInstance 	obj-firefox/xpcom/build/GenericFactory.cpp:16
7 	xul.dll 	nsComponentManagerImpl::CreateInstanceByContractID 	xpcom/components/nsComponentManager.cpp:1034
8 	xul.dll 	nsComponentManagerImpl::GetServiceByContractID 	xpcom/components/nsComponentManager.cpp:1426
9 	xul.dll 	nsCOMPtr_base::assign_from_gs_contractid 	obj-firefox/xpcom/build/nsCOMPtr.cpp:92
10 	xul.dll 	nsCOMPtr<nsINSSComponent>::nsCOMPtr<nsINSSComponent> 	obj-firefox/dist/include/nsCOMPtr.h:588
11 	xul.dll 	EnsureNSSInitialized 	security/manager/ssl/src/nsNSSComponent.cpp:304
12 	xul.dll 	`anonymous namespace'::nsCryptoHashConstructor 	security/manager/ssl/src/nsNSSModule.cpp:198
13 	xul.dll 	mozilla::GenericFactory::CreateInstance 	obj-firefox/xpcom/build/GenericFactory.cpp:16
14 	xul.dll 	nsComponentManagerImpl::CreateInstance 	xpcom/components/nsComponentManager.cpp:949
15 	xul.dll 	nsJSCID::CreateInstance 	js/xpconnect/src/XPCJSID.cpp:731
16 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
17 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1488
18 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:391
19 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2368
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsSSLIOLayerHelpers%3A%3ACleanup%28%29

Comment 1

[Josh Matthews [:jdm]]

I'll fix it, but probably not this week.

Comment 2

[Josh Matthews [:jdm]]

Attachment: Avoid performing cleanup on SSL state that doesn't exist.

Brian, this fixes a legitimate incorrect snippet. However, should we be worried that NSS initialization is apparently failing enough to make this error become a topcrash?

Comment 3

[Scoobidiver (away)]

With unpolluted crash stats (20.0a1 was), it's #28 browser crasher in 21.0a1 and there are currently no crashes in 20.0a2, so no longer a top crasher. Note that the ranking of a startup crash may vary a lot.

Comment 4

[Josh Matthews [:jdm]]

Two week review ping!

Comment 5

[Scoobidiver (away)]

Three-week review ping!!!!

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 701800 [details] [diff] [review]
Avoid performing cleanup on SSL state that doesn't exist.

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

Seems reasonable but it is unclear why we're getting here, which is concerning. Not sure what action to take about that bigger problem yet.

Comment 7

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57aed25c64eb

Comment 8

[Josh Matthews [:jdm]]

Comment on attachment 701800 [details] [diff] [review]
Avoid performing cleanup on SSL state that doesn't exist.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 769288
User impact if declined: Random crashes for some unknown set of users.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Zero.
String or UUID changes made by this patch: None.

Comment 9

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/mozilla-central/rev/57aed25c64eb

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 701800 [details] [diff] [review]
Avoid performing cleanup on SSL state that doesn't exist.

Low risk fix for a start-up crash in Fx20.
Verified that there are no crashes on nightly after this patch landed.

Comment 11

[Josh Matthews [:jdm]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a902a25121c8

Comment 12

[Paul Silaghi, QA [:pauly]]

No crashes on FF 21, 20 beta branches checking the crashstats for the last 4 weeks. Verified fixed.