Jan de Mooij [:jandem] Assignee
	Description • 6 years ago

Once bug 1466112 is finished, we'll have asserts in place to check ~nobody asks for the realm/global of a CCW (because it's meaningless since CCWs are shared by all realms).

However, internally the wrapper's ObjectGroup will still have a realm pointer and GC will trace group->realm->global so a potential risk is "leaking" globals that would have been dead if not for some CCW.

We can't just set group->realm to nullptr for CCW ObjectGroups, because then we also have no way to determine obj->compartment().

Some options:

(1) Make group->realm a RealmOrCompartment tagged pointer (JS::Realm* for most objects, JS::Compartment* for CCWs). This will require an extra branch on the obj->compartment() path but that's probably okay..

(2) Keep group->realm, but don't trace it for CCW groups. This makes it effectively a weak pointer - when we destroy a realm, we then have to go through all the wrappers in the compartment's wrapper map and change their group->realm to point at a realm in that compartment that's still alive. This gets complicated when all realms in the compartment are dead except for a CCW root.

(3) Have a dedicated wrapper realm.

The first one is probably the simplest to implement and to reason about.

Not high priority, but we should fix this at some point.

	Jan de Mooij [:jandem] Assignee
	Comment 1 • 6 years ago

(In reply to Jan de Mooij [:jandem] from comment #0)
> (1) Make group->realm a RealmOrCompartment tagged pointer (JS::Realm* for
> most objects, JS::Compartment* for CCWs). This will require an extra branch
> on the obj->compartment() path but that's probably okay..

...and this could be optimized when we know |obj| is definitely not a CCW.

	Jan de Mooij [:jandem] Assignee
	Comment 2 • 6 years ago

I've been thinking a bit about this lately. We should probably just allocate all CCWs in compartment->realms[0] for now. That way, worst-case that realm we keep alive a bit longer than necessary (when it only contains wrappers being used by other realms), but it's just a single realm and often the first realm will be the "main" realm anyway.

I'll write a patch for this soon.

	Jan de Mooij [:jandem] Assignee
	Comment 3 • 6 years ago

CCWs are not really associated with a single realm and we assert code doesn't
enter/request the realm or global of a CCW. However they currently still have an
ObjectGroup that's associated with a single realm. Using the same realm avoids
some potential memory usage issues.

	Pulsebot
	Comment 4 • 6 years ago

Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b67b31f6a81b
Always allocate CCWs in the compartment's first realm. r=jonco

	Razvan Maries
	Comment 5 • 6 years ago
bugherder

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