327564 - Hang involving E4X (cycle in an XML object?)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Comment 1

[Jesse Ruderman]

Attachment: testcase (hangs Firefox)

Comment 2

[Brendan Eich [:brendan]]

(In bug 312692 comment #6)
> Cycle detection means insuring that the would-be child can't reach the putative
> parent via a path in the DAG.  Yes, DAG: we allow
> 
>   parent = <p/>;
>   parent.child = "first";
>   child = parent.child[0];
>   parent.insertChildBefore(null, child);
> 
> to create a DAG with two arcs from parent to child, such that
> 
>   parent.child[0] === parent.child[1]

Now that ECMA-357 does not copy in appendChild and other insertions, you can end up with duplicate kids as shown here.  But if you then clobber parent.child via an assignment that sets a primitive tag content value, you can create a cycle that cannot be detected:

js> p = <p/>

js> p.c = 1
1
js> c = p.c[0]
1
js> p.insertChildBefore(null,c)
<p>
  <c>1</c>
  <c>1</c>
</p>
js> p.c[1] === c
true
js> p.c = 2
2
js> p
<p>
  <c>2</c>
</p>
js> c.appendChild(p)
// iloop here

E4X sucks.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 212234 [details] [diff] [review]
proposed fix

r=mrbkap

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 6

[Jesse Ruderman]

Now I see "Error: cyclic XML value" in my JavaScript console instead of a hang :)

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 212234 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz

Comment 8

[Brendan Eich [:brendan]]

Fixed on branches.

/be

Comment 9

[Bob Clary [:bc] (inactive)]

Checking in regress-327564.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-327564.js,v  <--  regress-327564.js
initial revision: 1.1
done

Comment 10

[Dave Liebreich [:davel]]

Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates) since in-testsuite+ indicates a test case exists in the js test library.

Comment 11

[Bob Clary [:bc] (inactive)]

v ff 1.8.0.1/1.8/1.9 20060302 win/linux/mac