1017150 - Breakpoint sweeping asserts with zone GCs

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: breakpoint-fix

Bug 1016738 causes us to do more zone GCs, and that patch uncovered this problem. Just because a debuggee compartment is being collected, we can't assume that the debugger compartment is also being collected.

Comment 1

[Jon Coppeard (:jonco)]

Comment on attachment 8430207 [details] [diff] [review]
breakpoint-fix

Review of attachment 8430207 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/gc/Zone.cpp
@@ +156,5 @@
>              Breakpoint *nextbp;
>              for (Breakpoint *bp = site->firstBreakpoint(); bp; bp = nextbp) {
>                  nextbp = bp->nextInSite();
> +                HeapPtrObject &dbgobj = bp->debugger->toJSObjectRef();
> +                JS_ASSERT_IF(dbgobj->zone()->isCollecting(), dbgobj->zone()->isGCSweeping());

Ah yes, this makes more sense.

@@ +161,2 @@
>                  bool dying = scriptGone || IsObjectAboutToBeFinalized(&dbgobj);
> +                JS_ASSERT_IF(!dying, IsAboutToBeFinalized(&bp->getHandlerRef()));

Did you mean to reverse the sense of this?  I agree with using IsAboutToBeFinalized() in preference to isMarked() though.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yes, I did reverse the isMarked test. Thanks for pointing it out.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3ce56241969

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

There seem to be several mochitest-other runs failing like this https://tbpl.mozilla.org/php/getParsedLog.php?id=40734131&tree=Mozilla-Inbound since this bug landed. Should I be concerned?

https://tbpl.mozilla.org/php/getParsedLog.php?id=40740817&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40740348&tree=Mozilla-Inbound

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm pretty sure the problem is in bug 1016738. It causes us to do more zone GCs, and there's probably some existing problem that gets triggered by that. Could you please back out 93c5b9181c84? I'm about to get on a plane.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Backed out for a spike in various GC-related crashes starting with this push. Sorry, I missed comment 4 before backing out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ac2205532ed

https://tbpl.mozilla.org/php/getParsedLog.php?id=40744831&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40744890&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40734131&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40735967&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40740817&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=40740348&tree=Mozilla-Inbound

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64788916250a

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/64788916250a