Improve Context shutdown

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 309f254c2c 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

.github/actions/install/action.yml

@@ -13,7 +13,7 @@ inputs:
   zig-v8:
     description: 'zig v8 version to install'
     required: false
-    default: 'v0.2.9'
+    default: 'v0.3.0'
   v8:
     description: 'v8 version to install'
     required: false

Dockerfile

@@ -3,7 +3,7 @@ FROM debian:stable-slim
 ARG MINISIG=0.12
 ARG ZIG_MINISIG=RWSGOq2NVecA2UPNdBUZykf1CCb147pkmdtYxgb3Ti+JO/wCYvhbAb/U
 ARG V8=14.0.365.4
-ARG ZIG_V8=v0.2.9
+ARG ZIG_V8=v0.3.0
 ARG TARGETPLATFORM
 
 RUN apt-get update -yq && \

build.zig.zon

@@ -6,8 +6,8 @@
     .minimum_zig_version = "0.15.2",
     .dependencies = .{
         .v8 = .{
-            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.2.9.tar.gz",
+            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.3.0.tar.gz",
-            .hash = "v8-0.0.0-xddH689vBACgpqFVEhT2wxRin-qQQSOcKJoM37MVo0rU",
+            .hash = "v8-0.0.0-xddH69R6BADRXsnhjA8wNnfKfLQACF1I7CSTZvsMAvp8",
         },
         //.v8 = .{ .path = "../zig-v8-fork" },
         .@"boringssl-zig" = .{

src/browser/Browser.zig

@@ -96,10 +96,6 @@ pub fn runMacrotasks(self: *Browser) !?u64 {
 }
 
 pub fn runMessageLoop(self: *const Browser) void {
-    while (self.env.pumpMessageLoop()) {
+    self.env.pumpMessageLoop();
-        if (comptime IS_DEBUG) {
-            log.debug(.browser, "pumpMessageLoop", .{});
-        }
-    }
     self.env.runIdleTasks();
 }

src/browser/js/Context.zig

@@ -43,6 +43,9 @@ env: *Env,
 page: *Page,
 isolate: js.Isolate,
 
+// Per-context microtask queue for isolation between contexts
+microtask_queue: *v8.MicrotaskQueue,
+
 // The v8::Global<v8::Context>. When necessary, we can create a v8::Local<<v8::Context>>
 // from this, and we can free it when the context is done.
 handle: v8.Global,
@@ -121,10 +124,6 @@ script_manager: ?*ScriptManager,
 // Our macrotasks
 scheduler: Scheduler,
 
-// Prevents us from enqueuing a microtask for this context while we're shutting
-// down.
-shutting_down: bool = false,
-
 unknown_properties: (if (IS_DEBUG) std.StringHashMapUnmanaged(UnknownPropertyStat) else void) = if (IS_DEBUG) .{} else {},
 
 const ModuleEntry = struct {
@@ -146,16 +145,11 @@ const ModuleEntry = struct {
 };
 
 pub fn fromC(c_context: *const v8.Context) *Context {
-    const data = v8.v8__Context__GetEmbedderData(c_context, 1).?;
+    return @ptrCast(@alignCast(v8.v8__Context__GetAlignedPointerFromEmbedderData(c_context, 1)));
-    const big_int = js.BigInt{ .handle = @ptrCast(data) };
-    return @ptrFromInt(big_int.getUint64());
 }
 
 pub fn fromIsolate(isolate: js.Isolate) *Context {
-    const v8_context = v8.v8__Isolate__GetCurrentContext(isolate.handle).?;
+    return fromC(v8.v8__Isolate__GetCurrentContext(isolate.handle).?);
-    const data = v8.v8__Context__GetEmbedderData(v8_context, 1).?;
-    const big_int = js.BigInt{ .handle = @ptrCast(data) };
-    return @ptrFromInt(big_int.getUint64());
 }
 
 pub fn deinit(self: *Context) void {
@@ -169,21 +163,15 @@ pub fn deinit(self: *Context) void {
             });
         }
     }
-    defer self.env.app.arena_pool.release(self.arena);
+
+    const env = self.env;
+    defer env.app.arena_pool.release(self.arena);
 
     var hs: js.HandleScope = undefined;
     const entered = self.enter(&hs);
     defer entered.exit();
 
-    // We might have microtasks in the isolate that refence this context. The
+    // this can release objects
-    // only option we have is to run them. But a microtask could queue another
-    // microtask, so we set the shutting_down flag, so that any such microtask
-    // will be a noop (this isn't automatic, when v8 calls our microtask callback
-    // the first thing we'll check is if self.shutting_down == true).
-    self.shutting_down = true;
-    self.env.runMicrotasks();
-
-    // can release objects
     self.scheduler.deinit();
 
     {
@@ -244,7 +232,13 @@ pub fn deinit(self: *Context) void {
             v8.v8__Global__Reset(global);
         }
     }
+
     v8.v8__Global__Reset(&self.handle);
+    env.isolate.notifyContextDisposed();
+    // There can be other tasks associated with this context that we need to
+    // purge while the context is still alive.
+    env.pumpMessageLoop();
+    v8.v8__MicrotaskQueue__DELETE(self.microtask_queue);
 }
 
 pub fn weakRef(self: *Context, obj: anytype) void {
@@ -992,13 +986,10 @@ pub fn queueSlotchangeDelivery(self: *Context) !void {
 // But for these Context microtasks, we want to (a) make sure the context isn't
 // being shut down and (b) that it's entered.
 fn enqueueMicrotask(self: *Context, callback: anytype) void {
-    self.isolate.enqueueMicrotask(struct {
+    // Use context-specific microtask queue instead of isolate queue
+    v8.v8__MicrotaskQueue__EnqueueMicrotask(self.microtask_queue, self.isolate.handle, struct {
         fn run(data: ?*anyopaque) callconv(.c) void {
             const ctx: *Context = @ptrCast(@alignCast(data.?));
-            if (ctx.shutting_down) {
-                return;
-            }
-
             var hs: js.HandleScope = undefined;
             const entered = ctx.enter(&hs);
             defer entered.exit();
@@ -1008,7 +999,8 @@ fn enqueueMicrotask(self: *Context, callback: anytype) void {
 }
 
 pub fn queueMicrotaskFunc(self: *Context, cb: js.Function) void {
-    self.isolate.enqueueMicrotaskFunc(cb);
+    // Use context-specific microtask queue instead of isolate queue
+    v8.v8__MicrotaskQueue__EnqueueMicrotaskFunc(self.microtask_queue, self.isolate.handle, cb.handle);
 }
 
 pub fn createFinalizerCallback(self: *Context, global: v8.Global, ptr: *anyopaque, finalizerFn: *const fn (ptr: *anyopaque) void) !*FinalizerCallback {

src/browser/js/Env.zig

@@ -249,10 +249,19 @@ pub fn createContext(self: *Env, page: *Page) !*Context {
     hs.init(isolate);
     defer hs.deinit();
 
+    // Create a per-context microtask queue for isolation
+    const microtask_queue = v8.v8__MicrotaskQueue__New(isolate.handle, v8.kExplicit).?;
+    errdefer v8.v8__MicrotaskQueue__DELETE(microtask_queue);
+
     // Get the global template that was created once per isolate
     const global_template: *const v8.ObjectTemplate = @ptrCast(@alignCast(v8.v8__Eternal__Get(&self.global_template, isolate.handle).?));
     v8.v8__ObjectTemplate__SetInternalFieldCount(global_template, comptime Snapshot.countInternalFields(Window.JsApi));
-    const v8_context = v8.v8__Context__New(isolate.handle, global_template, null).?;
+
+    const v8_context = v8.v8__Context__New__Config(isolate.handle, &.{
+        .global_template = global_template,
+        .global_object = null,
+        .microtask_queue = microtask_queue,
+    }).?;
 
     // Create the v8::Context and wrap it in a v8::Global
     var context_global: v8.Global = undefined;
@@ -292,6 +301,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context {
         .handle = context_global,
         .templates = self.templates,
         .call_arena = page.call_arena,
+        .microtask_queue = microtask_queue,
         .script_manager = &page._script_manager,
         .scheduler = .init(context_arena),
         .finalizer_callback_pool = std.heap.MemoryPool(Context.FinalizerCallback).init(self.app.allocator),
@@ -300,8 +310,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context {
 
     // Store a pointer to our context inside the v8 context so that, given
     // a v8 context, we can get our context out
-    const data = isolate.initBigInt(@intFromPtr(context));
+    v8.v8__Context__SetAlignedPointerInEmbedderData(v8_context, 1, @ptrCast(context));
-    v8.v8__Context__SetEmbedderData(v8_context, 1, @ptrCast(data.handle));
 
     try self.contexts.append(self.app.allocator, context);
     return context;
@@ -328,11 +337,13 @@ pub fn destroyContext(self: *Env, context: *Context) void {
     }
 
     context.deinit();
-    isolate.notifyContextDisposed();
 }
 
 pub fn runMicrotasks(self: *const Env) void {
-    self.isolate.performMicrotasksCheckpoint();
+    const v8_isolate = self.isolate.handle;
+    for (self.contexts.items) |ctx| {
+        v8.v8__MicrotaskQueue__PerformCheckpoint(ctx.microtask_queue, v8_isolate);
+    }
 }
 
 pub fn runMacrotasks(self: *Env) !?u64 {
@@ -360,12 +371,18 @@ pub fn runMacrotasks(self: *Env) !?u64 {
     return ms_to_next_task;
 }
 
-pub fn pumpMessageLoop(self: *const Env) bool {
+pub fn pumpMessageLoop(self: *const Env) void {
     var hs: v8.HandleScope = undefined;
     v8.v8__HandleScope__CONSTRUCT(&hs, self.isolate.handle);
     defer v8.v8__HandleScope__DESTRUCT(&hs);
 
-    return v8.v8__Platform__PumpMessageLoop(self.platform.handle, self.isolate.handle, false);
+    const isolate = self.isolate.handle;
+    const platform = self.platform.handle;
+    while (v8.v8__Platform__PumpMessageLoop(platform, isolate, false)) {
+        if (comptime IS_DEBUG) {
+            log.debug(.browser, "pumpMessageLoop", .{});
+        }
+    }
 }
 
 pub fn runIdleTasks(self: *const Env) void {

src/browser/js/Inspector.zig

@@ -241,8 +241,6 @@ pub const Session = struct {
             msg.ptr,
             msg.len,
         );
-
-        v8.v8__Isolate__PerformMicrotaskCheckpoint(isolate);
     }
 
     // Gets a value by object ID regardless of which context it is in.

src/browser/js/Isolate.zig

@@ -41,18 +41,6 @@ pub fn exit(self: Isolate) void {
     v8.v8__Isolate__Exit(self.handle);
 }
 
-pub fn performMicrotasksCheckpoint(self: Isolate) void {
-    v8.v8__Isolate__PerformMicrotaskCheckpoint(self.handle);
-}
-
-pub fn enqueueMicrotask(self: Isolate, callback: anytype, data: anytype) void {
-    v8.v8__Isolate__EnqueueMicrotask(self.handle, callback, data);
-}
-
-pub fn enqueueMicrotaskFunc(self: Isolate, function: js.Function) void {
-    v8.v8__Isolate__EnqueueMicrotaskFunc(self.handle, function.handle);
-}
-
 pub fn lowMemoryNotification(self: Isolate) void {
     v8.v8__Isolate__LowMemoryNotification(self.handle);
 }

src/browser/js/Local.zig

@@ -82,7 +82,7 @@ pub fn createTypedArray(self: *const Local, comptime array_type: js.ArrayType, s
 }
 
 pub fn runMicrotasks(self: *const Local) void {
-    self.isolate.performMicrotasksCheckpoint();
+    self.ctx.env.runMicrotasks();
 }
 
 // == Executors ==

src/cdp/cdp.zig

@@ -662,6 +662,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
 
         pub fn callInspector(self: *const Self, msg: []const u8) void {
             self.inspector_session.send(msg);
+            self.session.browser.env.runMicrotasks();
         }
 
         pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void {