759680 - crash in XPCWrappedNative::GetNewOrUsed @ JS_DHashTableOperate

Status: RESOLVED DUPLICATE of bug 756253

(Core :: XPConnect, defect)

Description

[Scoobidiver (away)]

It's #6 top crasher in 14.0b3.

Signature 	JS_DHashTableOperate More Reports Search
UUID	776e4b40-b617-4ea6-b87e-7eb8b2120530
Date Processed	2012-05-30 06:24:11
Uptime	524
Last Crash	2.1 days before submission
Install Age	2.7 days since version was first installed.
Install Time	2012-05-27 21:33:53
Product	FennecAndroid
Version	14.0
Build ID	20120524133102
Release Channel	beta
OS	Linux
OS Version	0.0.0 Linux 2.6.35.7-perf-I8150XXLM8-CL1088432 #3 PREEMPT Sat Apr 14 09:38:48 KST 2012 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterVendorID: qcom, AdapterDeviceID: GT-I8150.
AdapterDescription: 'Model: 'GT-I8150', Product: 'GT-I8150', Manufacturer: 'samsung', Hardware: 'qcom''.
samsung GT-I8150
samsung/GT-I8150/GT-I8150:2.3.6/GINGERBREAD/XXLM8:user/release-keys
EMCheckCompatibility	True

Frame 	Module 	Signature 	Source
0 	libxul.so 	JS_DHashTableOperate 	js/src/jsdhash.cpp:608
1 	libxul.so 	XPCWrappedNativeProto::GetNewOrUsed 	js/xpconnect/src/XPCMaps.h:383
2 	libxul.so 	ConstructSlimWrapper 	js/xpconnect/src/XPCWrappedNative.cpp:3899
3 	libxul.so 	XPCConvert::NativeInterface2JSObject 	js/xpconnect/src/XPCConvert.cpp:965
4 	libxul.so 	xpc_qsXPCOMObjectToJsval 	js/xpconnect/src/XPCQuickStubs.cpp:1034
5 	libxul.so 	nsIDOMDocument_CreateElement 	obj-firefox/js/xpconnect/src/dom_quickstubs.cpp:3314
6 	libxul.so 	js::Interpret 	js/src/jscntxtinlines.h:314
7 	libxul.so 	js::RunScript 	js/src/jsinterp.cpp:475
8 	libxul.so 	js::Execute 	js/src/jsinterp.cpp:674
9 	libxul.so 	JS_EvaluateUCScriptForPrincipalsVersionOrigin 	js/src/jsapi.cpp:5291
10 	libxul.so 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1458
11 	libxul.so 	nsScriptLoader::EvaluateScript 	content/base/src/nsScriptLoader.cpp:918
12 	libxul.so 	nsScriptLoader::ProcessRequest 	content/base/src/nsScriptLoader.cpp:811
13 	libxul.so 	nsScriptLoader::ProcessScriptElement 	content/base/src/nsScriptLoader.cpp:757
14 	libxul.so 	nsScriptElement::MaybeProcessScript 	content/base/src/nsScriptElement.cpp:169
15 	libxul.so 	nsIScriptElement::AttemptToExecute 	nsIScriptElement.h:253
16 	libxul.so 	nsHtml5TreeOpExecutor::RunScript 	parser/html/nsHtml5TreeOpExecutor.cpp:779
17 	libxul.so 	nsHtml5TreeOpExecutor::RunFlushLoop 	parser/html/nsHtml5TreeOpExecutor.cpp:583
18 	libxul.so 	nsHtml5ExecutorFlusher::Run 	parser/html/nsHtml5StreamParser.cpp:160
19 	libxul.so 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:656
20 	libxul.so 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:245
21 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:114
22 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:208
23 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:201
24 	libxul.so 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:189
25 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:295
26 	libxul.so 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3780
27 	libxul.so 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3857
28 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3933
...

More reports at:
https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=JS_DHashTableOperate

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

URLs:
 	http://www.firstrowsports.eu/watch/124184/1/watch-indiana-pacers-vs-miami-heat.h
2 	http://www.pbs.org/wgbh/masterpiece/watch/sherlock_belgravia.html
2 	http://www.erepublik.com/en
2 	about:blank
2 	http://video.pbs.org/video/2234656643
2 	http://www.blackplanet.com/user_search/index.html?tracking=global_nav
1 	http://www.hotnewhiphop.com/future-atl-dat-recipe-song.760666.html
1 	http://www.tvn24.pl/
1 	http://ssnif.333332.com/main.aspx
1 	https://www.paypal.com/cgi-bin/marketingweb?cmd=_login-run
1 	http://es.justin.tv/danser_tv1
1 	https://www.netbranch.app.fiserv.com/capedfcu/Default.aspx
1 	http://www.tvn24.pl/12690,1745664,0,1,na-a2-praca-wre-zdaza-na-euro,wiadomosc.ht
1 	http://jeux-video.fnac.com/a4092518/Game-of-Thrones-Le-Trone-de-Fer-Jeu-Xbox-360
1 	http://montreal.kijiji.ca/?rtlipmsg=1
1 	https://www.google.com/m?q=trashcan+minecraft&ie=utf-8&oe=utf-8&aq=t&rls=org.moz
1 	http://stewart.bookoo.com/
1 	https://apps.facebook.com/hidden-chronicles/?fb_source=dashboard_bookmark
1 	http://www.hotnewhiphop.com/future-atl-my-ho-2-prod-by-k-e-on-the-track-song.650
1 	http://www.arcadecandy.com/index.php?page=public.Game&gameId=3991
1 	http://universalsports.com/
1 	http://deals4mama.com/t1/index-luckydraw-1.php
1 	http://www.cnbeta.com/articles/189323.htm
1 	http://www.hotnewhiphop.com/sb-scaff-beezy-i-am-f1-song.684687.html
1 	http://vk.com/app109518_2914784?ref=1
1 	http://www.myfreecams.com/
1 	http://www.hotnewhiphop.com/miguel-adorn-song.686147.html
1 	http://www.droid-life.com/2012/05/26/motorola-demos-ice-cream-sandwich-for-the-d
1 	http://kotaku.com/5913203/the-internet-has-never-been-more-awkward-than-julia-st
1 	http://www.amazon.com/?ie=UTF8&ref=aw_bottom_links&force-full-site=1
1 	http://espn.go.com/watchespn/index/_/channel/espn3
1 	https://apps.facebook.com/thesimssocial/?
1 	http://www.hotnewhiphop.com/teairra-mari-u-did-dat-remix-song.659315.html
1 	http://kinglove.com/video/cbbc2d42c6bb2d486069959eaa9d9608.html?fid=Arab
1 	http://7f4yj.333332.com/main.aspx
1 	http://www.blackplanet.com/notes/view_note.html?note_id=1187565693&folder_id=2&f
1 	http://blog.sina.com.cn/imt4
1 	http://www.eurovision.tv/esctv/past?program=47113
1 	http://survey.acnielsenonline.com/wix1/p1421320720.aspx
1 	http://data.cnbc.com/quotes/fb
1 	http://komachi.yomiuri.co.jp/?from=yoltop

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Devices:
HTC EVO 3D X515m 	
Samsung SGH-I897 	
Samsung GT-I9100 	
Samsung SGH-I717 	
Samsung GT-I9000 	
HTC Glacier 	
HTC Vision

Comment 3

[JP Rosevear [:jpr]]

Bobby, the suggestion is this and bug 759674 and bug 759675 are XPC related.  Any ideas on how we could proceed on this top crashes for mobile?

Comment 4

[Bobby Holley (:bholley)]

(In reply to JP Rosevear [:jpr] from comment #3)
> Bobby, the suggestion is this and bug 759674 and bug 759675 are XPC related.
> Any ideas on how we could proceed on this top crashes for mobile?

Nothing jumps out at me. The maps are getting corrupted somehow and we're crashing while traversing them.

CCing mccr8, in case he has any ideas. But we're probably going to need STR, ideally on desktop.

Comment 5

[Andrew McCreight [:mccr8]]

Yeah, I'm not sure, sorry.

We should get JS_DHashTableOperate added to the skiplist thing.  Well, not the skip list, but the one where it still shows it.  PL_DHashTableOperate is on it already.

Comment 6

[Andrew McCreight [:mccr8]]

Every one of these crashes is a null-deref.  I looked at a random sample of 10 of these crashes and they are all at this line:
  keyHash = table->ops->hashKey(table, key);

That sounds to me like the hash table is uninitialized, rather than random corruption.

Bug 710922 had a similar problem.

Comment 7

[Andrew McCreight [:mccr8]]

The constructor for ClassInfo2WrappedNativeProtoMap actually initializes the hash table, so that can't be the problem.  In that case, my guess is that the call to JS_NewDHashTable in the constructor is returning null, which I think would also cause a null deref on the above line.  This can happen in two ways:

182     table = (JSDHashTable *) OffTheBooks::malloc_(sizeof *table);
183     if (!table)
184         return NULL;
185     if (!JS_DHashTableInit(table, ops, data, entrySize, capacity)) {
186         Foreground::free_(table);
187         return NULL;
188     }

Maybe it is running out of memory?

Comment 8

[Andrew McCreight [:mccr8]]

The thing I don't understand about this and bug 759674 is that the constructor is private, so I think the only way you can get a new instance of these classes is via newMap, which checks to make sure that mTable is non-null, so I'm not sure how you can end up with one of these maps with a null mTable!

Comment 9

[Andrew McCreight [:mccr8]]

Maybe it is possible that the entire map is null, and it is just showing up as being on this particular line due to whatever weird inlining is going on.  If newMap fails, then I think GetWrappedNativeProtoMap can return null, and I don't see a null check in XPCWrappedNativeProto::GetNewOrUsed.

Comment 10

[Nicholas Nethercote [inactive]]

> Bug 710922 had a similar problem.

Bug 710922 involved |HashTable|, which has a different implementation to |JSDHash|.

Comment 11

[Andrew McCreight [:mccr8]]

(In reply to Nicholas Nethercote [:njn] from comment #10)
> Bug 710922 involved |HashTable|, which has a different implementation to
> |JSDHash|.

Sure, but they both have the same weird intermediate state where they can be created but not initialized.  I believe js::HashTable is just a templatized version of JSDHash.

Comment 12

[Andrew McCreight [:mccr8]]

I did some hacky OOM testing by making the nth call to JS_NewDHashTable "fail" in  ClassInfo2WrappedNativeProtoMap's constructor by assigning null to mTable.  I tried a number of different values, and they all rapidly crashed after the fake OOM, in XPCWrappedNativeProto::GetNewOrUsed, which is where we are seeing these crashes.

I added a missing null check in XPCWrappedNativeProto::GetNewOrUsed, after the GetWrappedNativeProtoMap, but then it just ended up failing in JS_DHashTableEnumerate, inside XPCWrappedNativeScope::MarkAllWrappedNativesAndProtos.  I could null-check everything there, too, but maybe we should bail more completely earlier?  I have no idea what any of this code is actually trying to do. :)

Comment 13

[Bobby Holley (:bholley)]

Sounds like we should be using infallible malloc for allocating these maps. We're totally not set up to deal with allocation failures there.

Comment 14

[Andrew McCreight [:mccr8]]

Maybe it isn't that dire.  We could make XPCWrappedNativeScope::GetNewOrUsed detect if allocation of one of the hash tables failed, and return null in that case.  There are three places I can see that call XPCWrappedNativeScope::GetNewOrUsed.  Two of them check for a null return (xpc_CreateSandboxObject and nsXPConnect::InitClasses).  The third place (XPCWrappedNative::WrapNewGlobal) has assert, but no null check.  As far as I can see, we could just return an error there.  Of course, maybe it will all fail anyways.  I can poke at that.

Maybe we could adjust the size of the hash tables?  Though one is size 16 and the other 64, so they aren't amazingly huge.

Comment 15

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

STR : 
1. go to http://www.espncricinfo.com/
2. set flash to enabled
3. set text size to medium.
4. zoom and pan.

Expected: no crash
Actual: eventually you will crash

Nightly 06/06/2012 Samsung Galaxy SII

Comment 16

[Scoobidiver (away)]

I mixed the stack in comment 0 (JS_DHashTableOperate | XPCWrappedNativeProto::GetNewOrUsed - see https://crash-stats.mozilla.com/report/list?signature=JS_DHashTableOperate+|+XPCWrappedNativeProto%3A%3AGetNewOrUsed) and the bug summary and crash signature (JS_DHashTableOperate | XPCWrappedNative::GetNewOrUsed - see https://crash-stats.mozilla.com/report/list?signature=JS_DHashTableOperate+|+XPCWrappedNative%3A%3AGetNewOrUsed).
Are comments about the first one or the second one or are they the same thing?

Comment 17

[JP Rosevear [:jpr]]

Quite likely a dupe.

Comment 18

[Scoobidiver (away)]

There are no crashes in 14.0b7, probably fixed by bug 756253.

Comment 19

[Andrew McCreight [:mccr8]]

In 15/16 this and the 2 other similar bugs the signature is different now (due to bug 761249), but it sounds like it probably went away anyways.