890048 - GenerationalGC: Assertion failure: p, at shell/jsheaptools.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

findReferences([schedulegc(1)])

asserts js debug shell (tested with a threadsafe deterministic 64-bit debug build) on m-c changeset 85d75ed04851 without any CLI arguments at Assertion failure: p, at shell/jsheaptools.cpp

Comment 1

[Terrence Cole [:terrence]]

Attachment: Part 1 of 2: Fix DisableGenerationalGC and add EnableGenerationalGC api

FindReferences doesn't make much sense if the addresses are not stable. This patch fixes and adds api to enable and disable ggc at runtime programatically. The next patch in the series uses these to switch GGC off when we enter a HeapReverser and fixes the rooting of the objects HeapReverser stores.

Comment 2

[Terrence Cole [:terrence]]

Attachment: Part 2 of 2 - Fix HeapReverser for GGC.

In particular the use of |roots| seems wrong: it only tracks objects, but the cells in |map| and the Node lists can be any type. The attached patch rips out all the indirect rooting and just uses CustomAutoRooter's callback to manually mark everything. I think it's a bit more code overall, but much less cognitive overhead than the current scheme.

Comment 3

[Jim Blandy :jimb]

Comment on attachment 772388 [details] [diff] [review]
Part 2 of 2 - Fix HeapReverser for GGC.

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

Thank you so much.

::: js/src/shell/jsheaptools.cpp
@@ -165,5 @@
> -     * rule. This is kind of dumb, but JSAPI doesn't provide any less restricted
> -     * way to register arrays of roots.
> -     */
> -    Vector<jsval, 0, SystemAllocPolicy> roots;
> -    AutoArrayRooter rooter;

I love it.

@@ +245,5 @@
> +            e.front().value.trace(trc);
> +        }
> +        for (Child *c = work.begin(); c != work.end(); ++c)
> +            JS_CallGenericTracer(trc, c->cell, "HeapReverser::Child");
> +    }

Much nicer.

Comment 4

[Terrence Cole [:terrence]]

https://tbpl.mozilla.org/?tree=Try&rev=ba51f4e0ea40

https://hg.mozilla.org/integration/mozilla-inbound/rev/411082e7dc9c

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/411082e7dc9c