842272 - crash in mozilla::dom::SVGElementFactory::Shutdown @ nsComponentManagerImpl::IsContractIDRegistered

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Scoobidiver (away)]

It first showed up in 21.0a1/20130210. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=876652a0c8ef&tochange=29dd80c95b7d
It's likely a regression caused by bug 838501.

Signature 	PL_HashTableDestroy | nsComponentManagerImpl::IsContractIDRegistered(char const*, bool*) More Reports Search
UUID	17c546f4-0d37-407d-89d4-af33b2130210
Date Processed	2013-02-10 17:12:32
Uptime	0
Last Crash	4.1 weeks before submission
Install Age	48.8 minutes since version was first installed.
Install Time	2013-02-10 16:23:36
Product	FennecAndroid
Version	21.0a1
Build ID	20130210031150
Release Channel	nightly
OS	Android
OS Version	0.0.0 Linux 3.0.8+ #128 SMP PREEMPT Thu Dec 20 21:44:07 EST 2012 armv7l SAMSUNG/GT-P6800/GT-P6800:4.1.1/JRO03H/20121024.205236:eng/release-keys
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x8
App Notes 	
SAMSUNG N101 DUAL CORE
SAMSUNG/GT-P6800/GT-P6800:4.1.1/JRO03H/20121024.205236:eng/release-keys
Processor Notes 	sp-processor03.phx1.mozilla.com_15319:2008; WARNING: JSON file missing Add-ons; MDSW emitted too many frames, triggering truncation; exploitablity tool: ERROR: unable to analyze dump
EMCheckCompatibility	False	
Device	SAMSUNG N101 DUAL CORE
Android API Version	16 (REL)
Android CPU ABI	armeabi-v7a

Frame 	Module 	Signature 	Source
0 	libplds4.so 	PL_HashTableDestroy 	plhash.c:111
1 	libxul.so 	nsComponentManagerImpl::IsContractIDRegistered 	xpcom/components/nsComponentManager.cpp:1581
2 	libxul.so 	mozilla::dom::SVGElementFactory::Shutdown 	content/svg/content/src/SVGElementFactory.cpp:99
3 	libxul.so 	nsLayoutStatics::Shutdown 	layout/build/nsLayoutStatics.cpp:323
4 	libxul.so 	Initialize 	layout/build/nsLayoutStatics.h:44
5 	libnspr4.so 	PR_ExitMonitor 	ptsynch.c:557
6 	libxul.so 	nsComponentManagerImpl::KnownModule::Load 	xpcom/components/nsComponentManager.cpp:707
7 	libxul.so 	nsFactoryEntry::GetFactory 	xpcom/components/nsComponentManager.cpp:1748
8 	libxul.so 	nsComponentManagerImpl::CreateInstanceByContractID 	xpcom/components/nsComponentManager.cpp:1030
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=PL_HashTableDestroy+|+nsComponentManagerImpl%3A%3AIsContractIDRegistered%28char+const*%2C+bool*%29

Comment 1

[Robert Longson [:longsonr]]

I only have a Windows PC so I can't realistically look into this.

Comment 2

[Robert Longson [:longsonr]]

Any ideas Daniel?

Comment 3

[Robert Longson [:longsonr]]

Attachment: maybe we never got initialised?

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 715130 [details] [diff] [review]
maybe we never got initialised?

Yeah, it does look like there are some (unlikely) cases where we'd get a call to Shutdown() without ever having been initialized.

In particular: in nsLayoutModule.cpp's initialize() method, we call  nsLayoutStatics::Initialize(), which can return early -- and if it does return early (say, before it's gotten to the SVGElementFactory initialization), we immediately call shutdown(), which includes a call to SVGElementFactory::Shutdown.

So:
> void
> SVGElementFactory::Shutdown()
> {
>-  PL_HashTableDestroy(sTagAtomTable);
>+  if (sTagAtomTable) {
>+    PL_HashTableDestroy(sTagAtomTable);
>+  }
>   sTagAtomTable = nullptr;

Move the set-to-null inside the {}.

r=me with that

Comment 5

[Daniel Holbert [:dholbert]]

The sooner this lands the better, so that we don't have to worry about branch landings.

(tomorrow is merge day. I think normally it'd be today, except today's a holiday in the US.)

Comment 6

[Daniel Holbert [:dholbert]]

Landed (w/ comment 4 tweak) at Robert's request:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/162a860a2b39

(I'm pretty sure that'll end up being merged to m-c before the central-->aurora merge tomorrow. In the unlikely event that it's not, this patch should be able to get aurora approval without much trouble.)

Comment 7

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/162a860a2b39