Last Comment Bug 327564 - Hang involving E4X (cycle in an XML object?)
: Hang involving E4X (cycle in an XML object?)
Status: VERIFIED FIXED
[rft-dl]
: 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]
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 | Review

Description Jesse Ruderman 2006-02-16 16:47:16 PST
 
Comment 1 Jesse Ruderman 2006-02-16 16:47:49 PST
Created attachment 212168 [details]
testcase (hangs Firefox)
Comment 2 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
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] 2006-02-17 10:44:26 PST
Created attachment 212234 [details] [diff] [review]
proposed fix
Comment 4 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-02-17 13:24:15 PST
Comment on attachment 212234 [details] [diff] [review]
proposed fix

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

/be
Comment 6 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 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 Brendan Eich [:brendan] 2006-02-22 12:49:26 PST
Fixed on branches.

/be
Comment 9 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
done
Comment 10 Dave Liebreich [:davel] 2006-03-01 14:03:57 PST
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:] 2006-03-02 12:04:41 PST
v ff 1.8.0.1/1.8/1.9 20060302 win/linux/mac

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