627758 - jsd.off() fails intermittently

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

Internally, it periodically returns NS_ERROR_NOT_AVAILABLE.

Forked from bug 626830.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Take note of forced GC cancels

Transferring this patch from bug 626830.

Comment 2

[Steve Fink [:sfink] [:s:]]

Sorry for the torrent of bugspam, but I wanted to move over this description too:

The problem have some dead scripts that need to be discarded, but the JSGC_BEGIN callback cancels the GC run. JSD only remembers that it got a JSGC_BEGIN callback, and assumes that GC is still happening, so when jsdService::Off() is invoked, it barfs because you're not supposed to call it mid-GC. Unfortunately, it allows you to go ahead if there are no dead scripts lying around, which is why this failure is intermittent -- it depends on the relative timing of dead script creation and canceled GC runs.

This fixes the JSD bookkeeping. An alternative, broader fix would be to have GC
invoke the JSGC_END callback when a JSGC_BEGIN callback returns false,
cancelling the GC. That might fix other callers with similar assumptions,
though it's more of a behavior change than this patch.

Comment 3

[David Mandelin [:dmandelin]]

Comment on attachment 505832 [details] [diff] [review]
Take note of forced GC cancels

The comment could be more explicit. Something like "If we return false, then the GC will abort without making another callback with status=JSGC_END, so we set the status to JSGC_END here."

Comment 4

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/tracemonkey/rev/8743def9e748

Comment 5

[Steve Fink [:sfink] [:s:]]

Backed out due to template problem on Windows for a different bug.

http://hg.mozilla.org/tracemonkey/rev/32cf4e3c7a20

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8743def9e748
http://hg.mozilla.org/mozilla-central/rev/32cf4e3c7a20 (backout)
Note: not marking as fixed because last changeset is a backout.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Does anything still need to happen here? If not, can we checkin?

Comment 8

[Rob Campbell [:rc] (:robcee)]

looks like we need an updated patch to address whatever caused this patch to get backed out. This is still a hardblocker.

Comment 9

[Steve Fink [:sfink] [:s:]]

Actually, this patch was not to blame for the backout. But it is dependent on bug 626743 (the way debugging mode is entered will be changed pretty radically, so the fix here would need to be re-done.)

That bug is almost complete, so this should be able to go in soon.

Comment 10

[Steve Fink [:sfink] [:s:]]

Oh. I was thinking of a different patch. This one can go in independently.

http://hg.mozilla.org/tracemonkey/rev/c2f1e6f0d3a4

Comment 11

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/c2f1e6f0d3a4

Comment 12

[Boris Zbarsky [:bzbarsky]]

This was incorrectly cited as the bug in the checkin for bug 626830.