Closed Bug 582649 Opened 14 years ago Closed 14 years ago

Too-much-recursion crash with setUserData [@ * | XPCConvert::JSArray2Native]

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- .13-fixed
status1.9.1 --- .16-fixed

People

(Reporter: martijn.martijn, Assigned: peterv)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, Whiteboard: [sg:dos])

Crash Data

Attachments

(2 files, 1 obsolete file)

Testcase (crashes browser)
(deleted), text/html
Details
v1 (with crashtest)
(deleted), patch
peterv
: review+
benjamin
: approval2.0+
dveditz
: approval1.9.2.11-
dveditz
: approval1.9.2.13+
dveditz
: approval1.9.1.14-
dveditz
: approval1.9.1.16+
Details | Diff | Splinter Review
Attached file (deleted) — 
See unminimized testcase, you need to unzip it, then open the file named 'parentframe.htm'. And you need the script grant enhanced privileges (this is to force gc).
This is a typical crash, that I get:
http://crash-stats.mozilla.com/report/index/4c1f136e-d445-4630-ad33-1863f2100728
0  	xul.dll  	nsXPConnect::WrapJS  	 js/src/xpconnect/src/nsXPConnect.cpp:1348
1 	xul.dll 	XPCVariant::InitializeData 	js/src/xpconnect/src/xpcvariant.cpp:392
2 	xul.dll 	XPCVariant::newVariant 	js/src/xpconnect/src/xpcvariant.cpp:148
3 	xul.dll 	XPCConvert::JSData2Native 	js/src/xpconnect/src/xpcconvert.cpp:977
4 	xul.dll 	XPCConvert::JSArray2Native 	
5 	xul.dll 	XPCVariant::InitializeData 	
6 	xul.dll 	XPCConvert::JSData2Native 	js/src/xpconnect/src/xpcconvert.cpp:977
7 	xul.dll 	XPCConvert::JSArray2Native 	
8 	xul.dll 	XPCVariant::InitializeData 


Other crashes that I got:
http://crash-stats.mozilla.com/report/index/bp-202dd796-19f0-4e83-8359-165612100728
0  	xul.dll  	nsXPCWrappedJSClass::DelegatedQueryInterface  	 js/src/xpconnect/src/xpcwrappedjsclass.cpp:645
1 	xul.dll 	nsXPCWrappedJS::QueryInterface 	js/src/xpconnect/src/xpcwrappedjs.cpp:185
2 	xul.dll 	XPCConvert::JSObject2NativeInterface 	
3 	xul.dll 	xul.dll@0xa5eabb

http://crash-stats.mozilla.com/report/index/bp-148387f3-57ca-4d81-bd43-85f592100728
0  	xul.dll  	nsXPCWrappedJSClass::DelegatedQueryInterface  	 js/src/xpconnect/src/xpcwrappedjsclass.cpp:645
1 	xul.dll 	nsXPCWrappedJS::QueryInterface 	js/src/xpconnect/src/xpcwrappedjs.cpp:185
2 	xul.dll 	XPCConvert::JSObject2NativeInterface 	
3 	xul.dll 	xul.dll@0xa5eabb

Unfortunately, I wasn't able to minimize the testcase further, thus far.
Is this 4.0-only, or also 3.6.x?
The unminimized testcase only crashes on trunk, but that might perhaps only be because of the oddness of it.
For me this crashed in 3.6.x on OS X in nsCycleCollectingAutoRefCnt::get.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Attached file Testcase (crashes browser) (deleted) — 
This looks like it's caused by a recursive reference in an array. We recurse to death trying to wrap the array in XPConnect.
Attached patch v1 (obsolete) (deleted) — — Splinter Review 
Would it be ok to use the SpiderMonkey recursion checks in XPConnect?
Attachment #462151 - Flags: feedback?
Attachment #462151 - Flags: feedback? → feedback?(brendan)
Comment on attachment 462151 [details] [diff] [review]
v1

Brendan said "sure" on irc.
Attachment #462151 - Flags: feedback?(brendan) → review?(mrbkap)
Group: core-security
Keywords: testcase
Summary: Crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash [@ nsXPConnect::WrapJS]
Whiteboard: [sg:dos]
Attachment #462151 - Flags: review?(mrbkap) → review+
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash with setUserData
Summary: Too-much-recursion crash with setUserData → Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData → Too-much-recursion crash with setUserData [@ * | XPCConvert::JSArray2Native]
Should the patch get approval?
Attached patch v1 (with crashtest) (deleted) — — Splinter Review 
Fixes crash.
Attachment #462151 - Attachment is obsolete: true
Attachment #467752 - Flags: review+
Attachment #467752 - Flags: approval2.0?
Attachment #467752 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/f0f25f2693cd
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

Simple fix to catch recursion crash.
Attachment #467752 - Flags: approval1.9.2.10?
Attachment #467752 - Flags: approval1.9.1.13?
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Attachment #467752 - Flags: approval1.9.2.11?
Attachment #467752 - Flags: approval1.9.2.11+
Attachment #467752 - Flags: approval1.9.1.14?
Attachment #467752 - Flags: approval1.9.1.14+
Attachment #460910 - Attachment is private: true
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

missed the 1.9.2.11/1.9.1.14 releases, go for next time.
Attachment #467752 - Flags: approval1.9.2.12+
Attachment #467752 - Flags: approval1.9.2.11-
Attachment #467752 - Flags: approval1.9.2.11+
Attachment #467752 - Flags: approval1.9.1.15+
Attachment #467752 - Flags: approval1.9.1.14-
Attachment #467752 - Flags: approval1.9.1.14+
Crash Signature: [@ * | XPCConvert::JSArray2Native]
Depends on: 731334
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: