Closed Bug 627758 Opened 14 years ago Closed 14 years ago

jsd.off() fails intermittently

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: sfink, Assigned: sfink)

References

Details

(Whiteboard: ["Turn of debugger sync" changes are bug 626830][firebug-p1][hardblocker][fixed-in-tracemonkey])

Attachments

(1 file)

Internally, it periodically returns NS_ERROR_NOT_AVAILABLE. Forked from bug 626830.
Attached patch Take note of forced GC cancels (deleted) — Splinter Review
Transferring this patch from bug 626830.
Attachment #505832 - Flags: review?(dmandelin)
Assignee: general → sphink
blocking2.0: --- → ?
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.
Whiteboard: [firebug-p1]
blocking2.0: ? → betaN+
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."
Attachment #505832 - Flags: review?(dmandelin) → review+
Whiteboard: [firebug-p1] → [firebug-p1][hardblocker]
Status: NEW → ASSIGNED
Whiteboard: [firebug-p1][hardblocker] → [firebug-p1][hardblocker][fixed-in-tracemonkey]
Backed out due to template problem on Windows for a different bug. http://hg.mozilla.org/tracemonkey/rev/32cf4e3c7a20
Whiteboard: [firebug-p1][hardblocker][fixed-in-tracemonkey] → [firebug-p1][hardblocker]
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.
Does anything still need to happen here? If not, can we checkin?
Whiteboard: [firebug-p1][hardblocker] → [firebug-p1][hardblocker][has patch]
looks like we need an updated patch to address whatever caused this patch to get backed out. This is still a hardblocker.
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.
Depends on: 626743
Oh. I was thinking of a different patch. This one can go in independently. http://hg.mozilla.org/tracemonkey/rev/c2f1e6f0d3a4
Whiteboard: [firebug-p1][hardblocker][has patch] → [firebug-p1][hardblocker][fixed-in-tracemonkey]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
This was incorrectly cited as the bug in the checkin for bug 626830.
Whiteboard: [firebug-p1][hardblocker][fixed-in-tracemonkey] → ["Turn of debugger sync" changes are bug 626830][firebug-p1][hardblocker][fixed-in-tracemonkey]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: