Hang involving E4X (cycle in an XML object?)

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
JavaScript Engine
P1
critical
VERIFIED FIXED
12 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

12 years ago
 
(Reporter)

Comment 1

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

Comment 2

12 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

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

Comment 3

12 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

12 years ago
Fixed on trunk.

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

Comment 6

12 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

12 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

12 years ago
Fixed on branches.

/be
Keywords: fixed1.8.0.2, fixed1.8.1

Comment 9

12 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

12 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.