The v8 documentation says this "Has to be called", and claude flagged it as a
potential leak.
Required a new zig-v8-fork build (v0.5.5) which I released.
Mutable tags like @v6 can be re-pointed by whoever controls the action,
so the next run executes code nobody here reviewed, with the job's
token and secrets. Pinning to the full commit sha freezes what runs.
Version tags are kept as comments so renovate or dependabot can still
track updates.
dtolnay/rust-toolchain is pinned to a master commit with the toolchain
moved to an explicit input, as its readme recommends for sha pinning.
Depends on https://github.com/lightpanda-io/zig-v8-fork/pull/179
An improvement to https://github.com/lightpanda-io/browser/pull/2515 to prevent
a v8 assertion if we terminate as an inspector dispatch is happening.
The problem is that if we just immediately terminate, we aren't sure what the
worker thread is doing, and, apparently, if we terminate then dispatch a message
to the inspector, we fail an assertion.
With the way the code was, the only safe solution would be to hold a mutex
over the session dispatch, but that could block the network thread.
So instead of terminating from the network thread, we now ask v8 to execute
a callback. This gets executed on the worker thread, which can then terminate
the execution.
The initial version of 2515 delayed the termination from the network thread.
It's possible that solution would "solve" the issue, simply because it's very
unlikely that a worker would be "stuck" for 5 seconds and then get unstuck.
More likely that it exits immediately, or is stuck in an endless loop. But
that would still leave a window where we could terminate in network and then
dispatch in the worker. Less likely, but still possible. Hopefully this new
mechanism eliminates this from being a problem in all circumstances.
Under some conditions, a microtask would be executed for a context that was
already deinit'd, resulting in various use-after-free.
The culprit appears to be WASM compilation being placed in the microtask queue
(by a user-script) and then resolved at some point in the future. We guard the
microtask queue by a context.shutting_down boolean, but v8 doesn't know anything
about this flag. The fact is that, microtasks are tied to an isolate, not a
context.
This commit introduces a number of changes:
1 - It follows https://github.com/lightpanda-io/browser/commit/309f254c2c8c590a2655972aa1b31d020777634c and stores the zig Context inside of an embedder field. This
ensures v8 doesn't consider this when GC'ing, which _could_ extend the
lifetime of the v8::Context beyond what we expect
2 - Most significantly, it introduces per-context microtasks queues. Each
context gets its own queue. This makes cleanup much simpler and reduces the
chance of microtasks outliving the context
3 - pumpMessageLoop is called on context.deinit, this helps to ensure that any
tasks v8 has for our context are processed (e.g. wasm compilation) before
shtudown
4 - The order of context shutdown is important, we notify the isolate of the
context destruction first, then pump the message loop and finally destroy
the context's message loop.
Depends on https://github.com/lightpanda-io/zig-v8-fork/pull/151
Follow up to https://github.com/lightpanda-io/browser/pull/1495 which introduced
the concept of a fast getter. This commit expands the new behavior to all
comptime-known scalar getters.
It also leverages the new `v8__FunctionTemplate__New__Config` to
1 - flag fast getters as having no side effect
2 - set the length (arity) on all functions
Karl Seguin