331678 - jsxml.c needs to root better during QName creation

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Ignore the "ToAttributeName" at the bottom of that stack.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that I only saw one of the last two, but the other looked too similar not to patch the same way.

Comment 4

[Brendan Eich [:brendan]]

Attachment: 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

Comment 5

[Brendan Eich [:brendan]]

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

Comment 6

[Bob Clary [:bc] (inactive)]

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

Comment 7

[Daniel Veditz [:dveditz]]

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

Comment 8

[Brendan Eich [:brendan]]

Fixed on 1.8.0 branch.

/be

Comment 9

[Bob Clary [:bc] (inactive)]

David, do you have that e4x test case available?