Merge pull request #2584 from lightpanda-io/request_callback_terminate

Improve forced terminate on CDP client disconnect.

.github/actions/install/action.yml

@@ -13,7 +13,7 @@ inputs:
   zig-v8:
     description: 'zig v8 version to install'
     required: false
-    default: 'v0.4.5'
+    default: 'v0.4.6'
   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.4.5
+ARG ZIG_V8=v0.4.6
 ARG TARGETPLATFORM
 
 RUN apt-get update -yq && \

build.zig.zon

@@ -5,8 +5,8 @@
     .minimum_zig_version = "0.15.2",
     .dependencies = .{
         .v8 = .{
-            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.4.5.tar.gz",
-            .hash = "v8-0.0.0-xddH65CXBADLRFlCM_pECcwsoY-9P9mZ7VnYM-6V3mXW",
+            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.4.6.tar.gz",
+            .hash = "v8-0.0.0-xddH67-YBABDa7EPYFKQsWLdpK8xJvVVmDsKIaGPGID7",
         },
         // .v8 = .{ .path = "../zig-v8-fork" },
         .brotli = .{

src/browser/js/Env.zig

@@ -96,6 +96,10 @@ microtask_queues_are_running: bool,
 // IsExecutionTerminating check and PerformCheckpoint trips a V8 debug assert.
 terminate_mutex: std.Thread.Mutex = .{},
 
+// Set from network thread, saying termination should happen. Read from worker
+// thread making sure terminate hasn't been canceled.
+terminate_requested: std.atomic.Value(bool) = .init(false),
+
 pub const InitOpts = struct {
     with_inspector: bool = false,
 };
@@ -503,18 +507,38 @@ pub fn dumpMemoryStats(self: *Env) void {
     , .{ stats.total_heap_size, stats.total_heap_size_executable, stats.total_physical_size, stats.total_available_size, stats.used_heap_size, stats.heap_size_limit, stats.malloced_memory, stats.external_memory, stats.peak_malloced_memory, stats.number_of_native_contexts, stats.number_of_detached_contexts, stats.total_global_handles_size, stats.used_global_handles_size, stats.does_zap_garbage });
 }
 
+pub fn isExecutionTerminating(self: *const Env) bool {
+    return v8.v8__Isolate__IsExecutionTerminating(self.isolate.handle);
+}
+
 pub fn terminate(self: *Env) void {
     self.terminate_mutex.lock();
     defer self.terminate_mutex.unlock();
     v8.v8__Isolate__TerminateExecution(self.isolate.handle);
 }
 
+// Called from the network thread, caused v8 to eventually call terminateInterrupt
+pub fn requestTerminate(self: *Env) void {
+    self.terminate_requested.store(true, .release);
+    v8.v8__Isolate__RequestInterrupt(self.isolate.handle, terminateInterrupt, self);
+}
+
+// Runs on the worker thread
+fn terminateInterrupt(_: ?*v8.Isolate, data: ?*anyopaque) callconv(.c) void {
+    const self: *Env = @ptrCast(@alignCast(data.?));
+    if (self.terminate_requested.load(.acquire)) {
+        v8.v8__Isolate__TerminateExecution(self.isolate.handle);
+    }
+}
+
 /// Clears a pending termination so V8 calls (e.g. those made during cleanup)
 /// don't keep tripping over the terminating-state asserts. Safe to call
-/// unconditionally; a no-op if termination wasn't pending.
+/// unconditionally; a no-op if termination wasn't pending. Also clears the
+/// requestTerminate gate so any still-pending interrupt becomes a no-op.
 pub fn cancelTerminate(self: *Env) void {
     self.terminate_mutex.lock();
     defer self.terminate_mutex.unlock();
+    self.terminate_requested.store(false, .release);
     v8.v8__Isolate__CancelTerminateExecution(self.isolate.handle);
 }
 

src/cdp/CDP.zig

@@ -171,7 +171,7 @@ pub fn onLinkDisconnect(self: *CDP, err: ?anyerror) void {
 // Called by Network to try to force the Worker to shutdown. Protects against a
 // stuck worker.
 pub fn terminateFromNetwork(self: *CDP) void {
-    self.browser.env.terminate();
+    self.browser.env.requestTerminate();
 }
 
 // Called in the Worker to dispatch a single CDP message bubbled up by
@@ -182,6 +182,11 @@ pub fn terminateFromNetwork(self: *CDP) void {
 // frees right after we return), so `c.input`'s string slices stay
 // valid for the duration of dispatch.
 pub fn onMessage(self: *CDP, c: *Inbox.Message.Cdp) anyerror!void {
+    // Once a terminate is pending, don't dispatch
+    if (self.browser.env.isExecutionTerminating()) {
+        return;
+    }
+
     const arena = &self.message_arena;
     defer _ = arena.reset(.{ .retain_with_limit = 1024 * 16 });
     return self.dispatchParsed(arena.allocator(), .{ .cdp = self }, c.raw, c.input);