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

RESOLVED FIXED

Status

()

Core
DOM
--
critical
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: peterv)

Tracking

(Blocks: 1 bug, {crash, testcase})

unspecified
x86
Windows 7
crash, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 .13-fixed, status1.9.1 .16-fixed)

Details

(Whiteboard: [sg:dos], crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Created attachment 460910 [details]
zipped up unminimized testcase

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).
(Reporter)

Comment 1

7 years ago
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?
(Reporter)

Comment 3

7 years ago
The unminimized testcase only crashes on trunk, but that might perhaps only be because of the oddness of it.
(Assignee)

Comment 4

7 years ago
For me this crashed in 3.6.x on OS X in nsCycleCollectingAutoRefCnt::get.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
(Assignee)

Comment 5

7 years ago
Created attachment 462073 [details]
Testcase (crashes browser)

This looks like it's caused by a recursive reference in an array. We recurse to death trying to wrap the array in XPConnect.
(Assignee)

Comment 6

7 years ago
Created attachment 462151 [details] [diff] [review]
v1

Would it be ok to use the SpiderMonkey recursion checks in XPConnect?
Attachment #462151 - Flags: feedback?
(Assignee)

Updated

7 years ago
Attachment #462151 - Flags: feedback? → feedback?(brendan)
(Assignee)

Comment 7

7 years ago
Comment on attachment 462151 [details] [diff] [review]
v1

Brendan said "sure" on irc.
Attachment #462151 - Flags: feedback?(brendan) → review?(mrbkap)

Updated

7 years ago
Group: core-security
Keywords: testcase
Summary: Crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash [@ nsXPConnect::WrapJS]
Whiteboard: [sg:dos]

Updated

7 years ago
Attachment #462151 - Flags: review?(mrbkap) → review+

Updated

7 years ago
Duplicate of this bug: 475629

Updated

7 years ago
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash with setUserData
(Reporter)

Updated

7 years ago
Summary: Too-much-recursion crash with setUserData → Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData

Updated

7 years ago
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData → Too-much-recursion crash with setUserData [@ * | XPCConvert::JSArray2Native]

Comment 9

7 years ago
Should the patch get approval?
(Assignee)

Comment 10

7 years ago
Created attachment 467752 [details] [diff] [review]
v1 (with crashtest)

Fixes crash.
Attachment #462151 - Attachment is obsolete: true
Attachment #467752 - Flags: review+
Attachment #467752 - Flags: approval2.0?
Attachment #467752 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 11

7 years ago
http://hg.mozilla.org/mozilla-central/rev/f0f25f2693cd
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

7 years ago
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+
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+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/00c87ede773a
status1.9.2: --- → .13-fixed
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7da8e189105d
status1.9.1: --- → .16-fixed
Crash Signature: [@ * | XPCConvert::JSArray2Native]
Depends on: 731334
You need to log in before you can comment on or make changes to this bug.