The default bug view has changed. See this FAQ.

Hang involving E4X (cycle in an XML object?)

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
JavaScript Engine
P1
critical
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9alpha1
hang, testcase, verified1.8.0.2, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.0.2 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rft-dl])

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
 
(Reporter)

Comment 1

11 years ago
Created attachment 212168 [details]
testcase (hangs Firefox)
(Assignee)

Comment 2

11 years ago
(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
Assignee: general → brendan
Priority: -- → P1
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
OS: MacOS X → All
Hardware: Macintosh → All
(Assignee)

Comment 3

11 years ago
Created attachment 212234 [details] [diff] [review]
proposed fix
Attachment #212234 - Flags: superreview?(shaver)
Attachment #212234 - Flags: review?(mrbkap)
Comment on attachment 212234 [details] [diff] [review]
proposed fix

r=mrbkap
Attachment #212234 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 5

11 years ago
Fixed on trunk.

/be
Blocks: 309169
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: blocking1.8.1?
Flags: blocking1.8.0.2?
Resolution: --- → FIXED
(Reporter)

Comment 6

11 years ago
Now I see "Error: cyclic XML value" in my JavaScript console instead of a hang :)
Status: RESOLVED → VERIFIED
Attachment #212234 - Flags: superreview?(shaver) → superreview+
(Assignee)

Updated

11 years ago
Attachment #212234 - Flags: approval1.8.0.2?
Attachment #212234 - Flags: approval-branch-1.8.1+
Flags: blocking1.8.0.2? → blocking1.8.0.2+
Comment on attachment 212234 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz
Attachment #212234 - Flags: approval1.8.0.2? → approval1.8.0.2+
(Assignee)

Comment 8

11 years ago
Fixed on branches.

/be
Keywords: fixed1.8.0.2, fixed1.8.1

Comment 9

11 years ago
Checking in regress-327564.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-327564.js,v  <--  regress-327564.js
initial revision: 1.1
done
Flags: testcase+
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.
Whiteboard: [rft-dl]

Comment 11

11 years ago
v ff 1.8.0.1/1.8/1.9 20060302 win/linux/mac
Keywords: fixed1.8.0.2, fixed1.8.1 → verified1.8.0.2, verified1.8.1
(Assignee)

Updated

11 years ago
Flags: blocking1.8.1?
You need to log in before you can comment on or make changes to this bug.