Last Comment Bug 327564 - Hang involving E4X (cycle in an XML object?)
: Hang involving E4X (cycle in an XML object?)
: hang, testcase, verified1.8.0.2, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
P1 critical (vote)
: mozilla1.9alpha1
Assigned To: Brendan Eich [:brendan]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 326633 js1.6rc1
  Show dependency treegraph
Reported: 2006-02-16 16:47 PST by Jesse Ruderman
Modified: 2006-08-17 18:38 PDT (History)
5 users (show)
dveditz: blocking1.8.0.2+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (hangs Firefox) (173 bytes, text/html)
2006-02-16 16:47 PST, Jesse Ruderman
no flags Details
proposed fix (2.31 KB, patch)
2006-02-17 10:44 PST, Brendan Eich [:brendan]
mrbkap: review+
shaver: superreview+
brendan: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2006-02-16 16:47:16 PST
Comment 1 User image Jesse Ruderman 2006-02-16 16:47:49 PST
Created attachment 212168 [details]
testcase (hangs Firefox)
Comment 2 User image Brendan Eich [:brendan] 2006-02-16 20:01:41 PST
(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
js> c = p.c[0]
js> p.insertChildBefore(null,c)
js> p.c[1] === c
js> p.c = 2
js> p
js> c.appendChild(p)
// iloop here

E4X sucks.

Comment 3 User image Brendan Eich [:brendan] 2006-02-17 10:44:26 PST
Created attachment 212234 [details] [diff] [review]
proposed fix
Comment 4 User image Blake Kaplan (:mrbkap) 2006-02-17 13:24:15 PST
Comment on attachment 212234 [details] [diff] [review]
proposed fix

Comment 5 User image Brendan Eich [:brendan] 2006-02-17 13:40:27 PST
Fixed on trunk.

Comment 6 User image Jesse Ruderman 2006-02-17 18:48:28 PST
Now I see "Error: cyclic XML value" in my JavaScript console instead of a hang :)
Comment 7 User image Daniel Veditz [:dveditz] 2006-02-21 18:06:12 PST
Comment on attachment 212234 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz
Comment 8 User image Brendan Eich [:brendan] 2006-02-22 12:49:26 PST
Fixed on branches.

Comment 9 User image Bob Clary [:bc:] 2006-02-25 23:51:34 PST
Checking in regress-327564.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-327564.js,v  <--  regress-327564.js
initial revision: 1.1
Comment 10 User image Dave Liebreich [:davel] 2006-03-01 14:03:57 PST
Marking [rft-dl] (ready for testing in Firefox release candidates) since in-testsuite+ indicates a test case exists in the js test library.
Comment 11 User image Bob Clary [:bc:] 2006-03-02 12:04:41 PST
v ff 20060302 win/linux/mac

Note You need to log in before you can comment on or make changes to this bug.