1984091 - WeakRefs and FinalizationRegistries use CCWs that don't work with symbol targets

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P3)

Description

[Jon Coppeard (:jonco)]

Currently our implementation for these wraps the WeakRef/FinalizationRegistry object into the target zone. That doesn't work for symbol targets as the target zone is the atoms zone and we don't support allocating CCWs (or any object) in the atoms zone.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1984091 - Part 1: Add ObserverList data structure that holds a list of finalization observer objects r?sfink

Elements are added to these list on the main thread and removed by foreground
finalizers (also main thread). The elements will be WeakRefObjects and
FinalizationRecordObjcts (referred to as finalization observers).

Using this list removes the need to use cross compartment wrappers to refer to
these observers which is a problem because we can't create wrappers in the
atoms zone, where symbol targets live.

It also removes the need to have observers and targets in the same sweep group
which makes incremental sweeping less effective.

Finally it removes the overhead of allocating these extra wrapper objects.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1984091 - Part 2: Change WeakRefs to use the new ObserverList r?sfink

This removes wrapper creation, updating the cross-zone wrapper map for weak
refs and handling nuking of the wrappers.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1984091 - Part 3: Rewrite FinalizationRegistry to use ObserverList r?sfink

One effect of this change is to make this test code throw an exception when it
didn't before. I think what's happening is that nuking CCWs previously stopped
the finalization registry from calling the cleanup callback, whereas now this
happens and an exception is correctly thrown. This doesn't have any effect on
the original purpose of the testcase which was to trigger an over-eager
assertion, or correctness of the finalization regisry implementation.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/433d5b2cc9ef
https://hg.mozilla.org/integration/autoland/rev/3cb12b48b1c5
Part 1: Add ObserverList data structure that holds a list of finalization observer objects r=sfink
https://github.com/mozilla-firefox/firefox/commit/0e4533154def
https://hg.mozilla.org/integration/autoland/rev/72fcb6edb68a
Part 2: Change WeakRefs to use the new ObserverList r=sfink
https://github.com/mozilla-firefox/firefox/commit/0d2813ea4727
https://hg.mozilla.org/integration/autoland/rev/064fe5443ae5
Part 3: Rewrite FinalizationRegistry to use ObserverList r=sfink
https://github.com/mozilla-firefox/firefox/commit/447d0b5a2e48
https://hg.mozilla.org/integration/autoland/rev/70117386c757
apply code formatting via Lando

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3cb12b48b1c5
https://hg.mozilla.org/mozilla-central/rev/72fcb6edb68a
https://hg.mozilla.org/mozilla-central/rev/064fe5443ae5
https://hg.mozilla.org/mozilla-central/rev/70117386c757

Comment 6

[Tom S. (please needinfo tschuster)]

Can you send an Intent to Ship?

Comment 7

[Jon Coppeard (:jonco)]

This bug is preparatory work. The actual change is in bug 1863140, and will be preffed off to start with. But thanks for reminding me, I will send an intent to ship before turning this on.