Note: There are a few cases of duplicates in user autocompletion which are being worked on.

jsxml.c needs to root better during QName creation

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: dbaron, Assigned: brendan)

Tracking

({fixed1.8.0.4, fixed1.8.1})

Trunk
x86
Linux
fixed1.8.0.4, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.1 ?
blocking1.8.0.4 +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

12 years ago
I have an E4X testcase that I just ran under WAY_TOO_MUCH_GC, and hit a few GC hazards in jsxml.c.  The first one's obvious (js_NewObject can do last-ditch allocation), and the stack where the second one can cause GC is:

js_GC (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:2148)
js_NewGCThing (/home/dbaron/builds/trunk/mozilla/js/src/jsgc.c:668)
js_NewObject (/home/dbaron/builds/trunk/mozilla/js/src/jsobj.c:2021)
NewXMLObject (/home/dbaron/builds/trunk/mozilla/js/src/jsxml.c:7338)
js_GetXMLObject (/home/dbaron/builds/trunk/mozilla/js/src/jsxml.c:7373)
GetProperty (/home/dbaron/builds/trunk/mozilla/js/src/jsxml.c:4062)
ToAttributeName

I'll attach a patch; the first chunk is rather odd.  Perhaps it should be done with locking instead.
(Reporter)

Comment 1

12 years ago
Ignore the "ToAttributeName" at the bottom of that stack.
(Reporter)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: blocking1.8.1?
Flags: blocking1.8.0.3?
Whiteboard: [patch]
(Reporter)

Comment 2

12 years ago
Created attachment 216199 [details] [diff] [review]
patch
Attachment #216199 - Flags: review?(brendan)
(Reporter)

Comment 3

12 years ago
Note that I only saw one of the last two, but the other looked too similar not to patch the same way.
(Assignee)

Comment 4

12 years ago
Created attachment 216238 [details] [diff] [review]
use a local root if possible, plus style nits

Thanks dbaron, I'll take this on if you don't mind since it's all my fault.

Nits: /* Major comments use sentences and have one blank line in front. */; s/JSBool result/JSBool ok/.

/be
Assignee: dbaron → brendan
Attachment #216199 - Attachment is obsolete: true
Attachment #216238 - Flags: review?(mrbkap)
Attachment #216238 - Flags: approval1.8.0.3?
Attachment #216238 - Flags: approval-branch-1.8.1+
Attachment #216199 - Flags: review?(brendan)

Updated

12 years ago
Attachment #216238 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 5

12 years ago
Fixed on trunk and 1.8 branch.

Bob, I forgot to link the last jsxml.c-patching bug up to js1.6rc1, but I don't know where rc1 stands.  Neither is a blocker for rc1, but both could go into js1.6 and do the world some favors.  Comments?

/be
Blocks: 309169
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED

Comment 6

12 years ago
(In reply to comment #5)

I'll get them in. I will grab the patches from the latest additions today, confirm they build then cut the branch later today. I have been remiss in getting rc1 out the door but will push hard today.
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Comment on attachment 216238 [details] [diff] [review]
use a local root if possible, plus style nits

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

Comment 8

11 years ago
Fixed on 1.8.0 branch.

/be
Keywords: fixed1.8.0.3

Comment 9

11 years ago
David, do you have that e4x test case available?

Updated

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