475629 - Crash due to too much recursion with XPCConvert::JSArray2Native

Status: RESOLVED DUPLICATE of bug 582649

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: testcase

...
23  XPCConvert::JSArray2Native
24  XPCVariant::InitializeData
25  XPCVariant::newVariant
26  XPCConvert::JSData2Native
27  XPCConvert::JSArray2Native
...

Comment 1

[Carsten Book [:Tomcat]]

testcase crashes 3.0.14 builds on load http://crash-stats.mozilla.com/report/index/23ca5285-89eb-46d9-ac56-bccda2090915?p=1 up to 1.9.2 Nightly builds -> http://crash-stats.mozilla.com/report/index/8a93af91-8ea6-4b45-9d21-ad99a2090915?p=1

Comment 2

[timeless]

i think that what should happen is that xpconnect should register the original variant in its map

and before it converts the next variant it encounters it should see if the variant is already in the map

and if it is, it should just grab it

right now, it's just treating each variant it encounters as something it has to marshall from scratch

i think that might mean using nsXPCWrappedJS::GetNewOrUsed instead of XPCVariant::newVariant

Comment 3

[David Bradley]

I think it's a little more complex than that. In a = []; a[0] = a; We haven't created the native for "a" yet, so we can't populate element 0. Ideally we need to assign element 0 after the array is built.

I wonder if we unrolled the recursion that exists in JS to native conversion. Not sure if that would help or not.

Comment 4

[David Bradley]

Also was thinking of a way to maybe prevent this or hack something together to address it for the a = []; a[0] = a; case. But there are more tricky cases such as:

  var a = [];
  var b = [];
  a[0] = b;
  b[0] = a;

Comment 5

[timeless]

well, i think we should basically create a basic object and register it immediately such that when we start looking for objects we have a chance to find them. Rough outline:

133 XPCVariant* XPCVariant::newVariant(XPCCallContext& ccx, jsval aJSVal)
144     NS_ADDREF(variant);
+ TIE_JSVAL_TO_XPCONNECT(aJSVal, variant); /* register immediately */
146     if(!variant->InitializeData(ccx))
/* the array is populated here */

495 XPCConvert::JSData2Native(XPCCallContext& ccx, void* d, jsval s,
961         case nsXPTType::T_INTERFACE_IS:
966             if(iid->Equals(NS_GET_IID(nsIVariant)))

-968                 XPCVariant* variant = XPCVariant::newVariant(ccx, s);
+XPCVariant* variant = nsnull;
+SOMETHING* wrapper = nsnull;
+nsXPCWrappedJS::GetUsedOnly(ccx, iid, s, &wrapper); /* look for objects */
+if (wrapper) {
+ getVariantFromWrapper(wrapper, &variant);
+}
+if (!variant) {
+ variant = XPCVariant::newVariant(ccx, s);
+}

Sadly, nsXPCWrappedJS::GetUsedOnly doesn't exist.