850849 - Various fixes for WeakMap for generational GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: v0

There are three fixes here. The first is to only ExposeToActiveJS on tenured things. The second is to change the type pointer we mark to be raw instead of an encapsulated pointer -- if the pre-barrier triggers incidentally, than all sorts of bad things happen. The third is to fix the marking of WeakMap objects themselves (rather than a contained thing) when doing a generational GC.

Comment 1

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

Comment on attachment 724597 [details] [diff] [review]
v0

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

::: js/src/jsweakmap.cpp
@@ +211,5 @@
>          if (ObjectValueMap::Ptr ptr = map->lookup(key)) {
>              // Read barrier to prevent an incorrectly gray value from escaping the
>              // weak map. See the comment before UnmarkGrayChildren in gc/Marking.cpp
> +            if (ptr->value.isMarkable() && !gc::IsInsideNursery(cx->runtime, ptr->value.toGCThing()))
> +                ExposeValueToActiveJS(ptr->value.get());

Yikes, this is going to be a problem for the browser. I guess this is okay for now.

@@ +325,5 @@
>      }
> +
> +    typedef WeakMap<JSObject *, Value> UnbarrieredObjectValueMap;
> +    UnbarrieredObjectValueMap *unbarriered = reinterpret_cast<UnbarrieredObjectValueMap *>(map);
> +    HashTableWriteBarrierPost(cx->runtime, unbarriered, key.get());

The same comment applies as in bug 850954. If the suggestion there doesn't work out, you can land this as part of that bug.

Comment 2

[Terrence Cole [:terrence]]

Both blocking comments are fixed now:

https://hg.mozilla.org/integration/mozilla-inbound/rev/616f9e03bfb2

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/616f9e03bfb2