1300210 - Initialize nsCCUncollectableMarker::sGeneration to 1

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

Otherwise we treat our GCs before the first CC like shutdown which is weird.

Comment 1

[Andrew McCreight [:mccr8]]

The immediate reason we need to fix this is that Terrence added a verifier that there are no black-gray edges in the JS heap, and this causes that to fail, due to mozilla::dom::TraceBlackJS(). If something is a black root, I believe the verifier requires that it be marked black. This makes sense, because a black root is something that is definitely alive, and if the object is marked gray, the CC might free it.

However, Boris was seeing a case where the browser starts, we GC and mark the whatever stuff at the bottom of TraceBlackJS grey. Then we run the CC, which flips sGeneration to 1. Now, the verifier runs, and it sees that the stuff in TraceBlackJS claims to be black, but is grey, causing a failure.

In this particular case the code is actually safe. The purpose of the black-gray invariant is to ensure that the CC does not incorrectly free any gray objects that are reachable from black JS roots. The objects in TraceBlackJS should all be reachable directly from a C++ object, and the CC knows that those C++ objects are alive (because of refcounting). Therefore, the CC will not unlink any objects that are reachable from black JS roots.

Anyways, the easiest fix here is to simply start at 1 instead of 0.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1300210 - Initialize nsCCUncollectableMarker::sGeneration to 1.

The immediate reason we need to change this is that bug 1299911
proposes adding a verifier to check that there are no black-gray edges
in the JS heap, and sGeneration being 0 causes that to fail, due to
mozilla::dom::TraceBlackJS(). If something is a black root, I believe
the verifier requires that it be marked black. This makes sense,
because a black root is something that is definitely alive, and if the
object is marked gray, the CC might free it, as far as the GC knows.

This fails because when the browser starts, it GCs and marks the stuff
at the bottom of TraceBlackJS grey. Then it runs the CC, which flips
sGeneration to 1. Now, the verifier runs (before the GC runs!), and it
sees that the stuff in TraceBlackJS claims to be black, but is grey,
causing a verification failure.

In this particular case the code is actually safe. The purpose of the
black-gray invariant is to ensure that the CC does not incorrectly
unlink any gray C++ objects that are reachable from black JS
roots. The JS objects in TraceBlackJS should all be reachable directly
from a C++ object, and the CC knows that those C++ objects are alive
(because of refcounting). Therefore, the CC will not unlink any
objects that are reachable from black JS roots.

Review commit: https://reviewboard.mozilla.org/r/76480/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76480/

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8787795 [details]
Bug 1300210 - Initialize nsCCUncollectableMarker::sGeneration to 1.

https://reviewboard.mozilla.org/r/76480/#review74582

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf059ec2bdc9
Initialize nsCCUncollectableMarker::sGeneration to 1. r=smaug

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bf059ec2bdc9