From 86b9e8675e46eed7b8e58cae1d3b63e70e1e8c29 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Fri, 5 Jun 2026 13:00:35 +0800
Subject: [PATCH] Make worker more robust in the face of a disconnected CDP
 client

When a CDP client disconnects, through a series of step, we call `Isolate::
TerminateExecution`. The problem is that this only terminates the currently
executing script. If scripts are nested, or another script runs immediately
after the terminated one, the worker keeps running.

I was specifically trying to fix some 100% endless loop in a few WPT tests, e.g
Worker-terminate-forever-during-evaluation.html

The first is the issue described above. We potentially need to guard every entry
into script executing with `if (!env.is_terminated)`. AND/OR we need to bubble
an error when a script _is_ terminated (right now, most JS errors map to
error.JsException which makes this hard). For the specific WPT failures, I opted
for a more targeted fix directly in the Worker. But I think we need to keep an
eye out on other cases where this can happen.

I also discovered another issue, which is not addressed is this commit:

var worker = new Worker("endlessloop.js");
worker.terminate(); <-- never gets called

Given our current architecture, the only solution I can think of is a watcher
thread (or re-using the main/network thread).
---
 src/browser/js/Env.zig        |  8 ++++++++
 src/browser/webapi/Worker.zig | 18 ++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig
index 1fa807f8..97d093fb 100644
--- a/src/browser/js/Env.zig
+++ b/src/browser/js/Env.zig
@@ -514,6 +514,14 @@ pub fn isExecutionTerminating(self: *const Env) bool {
     return v8.v8__Isolate__IsExecutionTerminating(self.isolate.handle);
 }
 
+// Whether a forcible terminate has been requested (and not yet cleared by
+// cancelTerminate). Unlike isExecutionTerminating, this is our own sticky
+// flag, so it stays true after V8 consumes the terminate on the JSEntry
+// unwind. Callers about to enter a fresh eval use it to refuse to run.
+pub fn terminatePending(self: *const Env) bool {
+    return self.terminate_requested.load(.acquire);
+}
+
 pub fn terminate(self: *Env) void {
     self.terminate_mutex.lock();
     defer self.terminate_mutex.unlock();
diff --git a/src/browser/webapi/Worker.zig b/src/browser/webapi/Worker.zig
index a9b1b0c9..5216fe85 100644
--- a/src/browser/webapi/Worker.zig
+++ b/src/browser/webapi/Worker.zig
@@ -182,6 +182,12 @@ fn httpDoneCallback(ctx: *anyopaque) !void {
 }
 
 fn loadInitialScript(self: *Worker, script: []const u8) !void {
+    const js_context = self._worker_scope.js;
+
+    if (js_context.env.terminatePending()) {
+        return;
+    }
+
     // Keep buffering throughout the entire outer eval (including any
     // runMacrotasks pumped by importScripts via the synchronous CDP path,
     // see WorkerGlobalScope.importScripts). The flip-and-drain happens
@@ -200,7 +206,7 @@ fn loadInitialScript(self: *Worker, script: []const u8) !void {
     }
 
     var ls: js.Local.Scope = undefined;
-    self._worker_scope.js.localScope(&ls);
+    js_context.localScope(&ls);
     defer ls.deinit();
 
     var try_catch: js.TryCatch = undefined;
@@ -213,12 +219,20 @@ fn loadInitialScript(self: *Worker, script: []const u8) !void {
     // synchronously through ScriptManagerBase (client.tick sync_wait).
     switch (self._type) {
         .classic => _ = ls.local.eval(script, self._url) catch |err| {
+            if (js_context.env.terminatePending()) {
+                return;
+            }
+
             const caught = try_catch.caughtOrError(self._arena, err);
             log.err(.browser, "worker script error", .{ .url = self._url, .caught = caught });
             self.fireErrorEvent(caught.exception orelse @errorName(err), null);
             return;
         },
-        .module => self._worker_scope.js.module(false, &ls.local, script, self._url, true) catch |err| {
+        .module => js_context.module(false, &ls.local, script, self._url, true) catch |err| {
+            if (js_context.env.terminatePending()) {
+                return;
+            }
+
             const caught = try_catch.caughtOrError(self._arena, err);
             log.err(.browser, "worker module error", .{ .url = self._url, .caught = caught });
             self.fireErrorEvent(caught.exception orelse @errorName(err), null);