The point of this change is that v8 might call these finalizers after we've
cleared things (e.g. because it's already queued it). We need to make the
FC.Identity self-contained and not rely on any external state (like the
finalizer_callback lookup) which might have new entries.
2 small changes:
1 - Ensure that isolated world identity is always reset. Not clear how we can
have identity without a context, but very little harm in doing it this way.
2 - Clear finalizer_callback map _before_ finalizing an instance. Finalizing
an instance could (a) release an arena (they almost all do) and (b) create an
object with the newly released arena. I don't think anything does that now, but
they could. That object could even be passed into v8 during finalization. In
which case, we'd temporarily have 2 "live" instances (one being finalized, one
jsut created) at the same address. Don't think we have any code that does this,
but switching the order (remove from map, then finalize) protects against this
address re-use.
1 big change:
Not really big, but more likely to actually fix things. A finalizer can still
be called _after_ we've cleared the finalizer callback. This can happen if
v8 has queued the finalizer prior to us clearing it. We do see some evidence
that this might be an issue, as many extra releaseRefs are happening in
the message loop or microtasks. This makes that scenario safer. First, it moves
the finalizer identity to a dedicated MemoryPool that can outlive the page.
Second, it uses the finalizer_callback map itself to tell whether or not
anything has to happen.
This is an attempt to fix reference counting issues. When Session.replacePage
is called, isolated worlds survive. This appears to be the correct behavior, but
it means that their identity outlives the page reset, which can result in a
use-after-free. The idea is that, on reset, IsolatedWorld persist, but their
identity is cleared.
It should be impossible for a internal cache get to have an incorrect # of
internal fields. But we're seeing this exact scenario in production.
https://github.com/lightpanda-io/browser/pull/1991 was meant to help with this,
but you can do some pretty weird things in JavaScript and it's possible there's
some combination of JavaScript which still allows calling these methods on a
different receiver.
1. Double buffer to_load list so that load callback which register more loadable
elements don't invalid the list while we're iterating
2. Switch to debug-only assertion for opaque origin. Not clear how this
assertion is failing, but isn't worth failing release builds for it.
3. on worker terminate, don't remove worker from page tracking. This results in
a leaking context, which causes numerous problems.
4. On page.init error, cleanly shutdown context
Fetch is owned by response.arena (a) we need to clear the flag before freeing
the arena and (b) there's no point in clearing the flag at all, since the
memory is freed.
In order for an API to be supported by workers, their dependency on *Page has
to be removed. To keep this PR smaller, we're only converting a minimum number
of APIs from Page to Execution. All other APIs should not be exposed to the
worker (better to get a FormData undefined than to try to segfault trying to
execute FormData without a page).
Turns out you can embed multiple contexts within a snapshot. So our snapshot
now contains the Page context (as before) but also the Worker context. This
gives us the performance benefit of snapshots and makes context creation for
pages and workers much more similar.
We'll have two types of contexts: one for pages and one for workers. They'll
[probably] both be js.Context, but they'll have distinct FunctionTemplates
attached to their global. The Worker template will only contain a small subset
of the main Page's types, along with 1 or 2 of its own specific ones.
The Snapshot now creates the templates for both, so that the Env contains the
function templates for both contexts. Furthermore, having a "merged" view like
this ensures that the env.template[N] indices are consistent between the two.
However, the snapshot only attaches the Page-specific types to the snapshot
context. This allows the Page-context to be created as-is (e.g. efficiently).
The worker context will be created lazily, on demand, but from the templates
loaded into the env (since, again, the env contains templates for both).
A Worker has no page. So any API that is accessible to a worker cannot take
a *Page parameter. Such APIs will now take a js.Execution which the context
will own and create from the Page (or from the WorkerGlobalScope when that's
created).
To test this, in addition to introducing the Execution, this change also updates
URLSearchParams which is accessible to Worker (and the Page obviously). This
change is obviously viral..if URLSearchParams no longer has a *Page but instead
has an *Execution, then any function it calls must also be updated.
So some APIs will take a *Page (those only accessible from a Page) and some will
take an *Execution (those accessible from a Page or Worker). I'm ok with that.
A lot of private/internal functions take a *Page, because it's simple, but all
they want is a call_arena or something. We'll try to update those as much as
possible. The Page/Execution being injected from the bridge is convenient, but
we should be more specific for internal calls and pass only what's needed.
