From 9c1998076cd8627afd84aed0f81ab09226ca7fbf Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 13 Aug 2026 14:07:32 +0800 Subject: [PATCH] stability: cancel a termination only at a safe point Cancel termination only when all context have a call_depth == 0. This ensures that termination "sticks" all the way up a nested call. Without this, a nested call could clear a terminate which targeted something up the stack. --- src/browser/js/Env.zig | 28 ++++---- src/browser/js/Function.zig | 124 ++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 17 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index af2cfa925..c42f837d0 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -427,25 +427,24 @@ pub fn runMicrotasks(self: *Env) void { while (i < self.contexts.items.len) : (i += 1) { const ctx = self.contexts.items[i]; v8.v8__MicrotaskQueue__PerformCheckpoint(ctx.microtask_queue, v8_isolate); + if (self.terminatePending()) { - // A terminate landed inside this checkpoint. Unlike a JS entry, - // the checkpoint exit does not clear V8's terminating state, and - // leaving it set makes later V8 calls (teardown, inspector) fail - // in ways their callers don't expect. Clear the V8 state; the - // sticky terminate_requested keeps blocking new JS entries. - clearTerminationState(v8_isolate); + if (v8.v8__Isolate__IsExecutionTerminating(v8_isolate)) { + for (self.contexts.items) |c| { + if (c.call_depth > 0) { + return; + } + } + // None of the contexts are "entered", it's safe to + // clear the termination flag. + v8.v8__Isolate__CancelTerminateExecution(v8_isolate); + } return; } } } } -fn clearTerminationState(v8_isolate: *v8.Isolate) void { - if (v8.v8__Isolate__IsExecutionTerminating(v8_isolate)) { - v8.v8__Isolate__CancelTerminateExecution(v8_isolate); - } -} - pub fn runMacrotasks(self: *Env) !void { if (self.terminatePending()) { return; @@ -648,11 +647,6 @@ pub fn performIsolateMicrotasks(self: *Env) void { defer self.terminate_mutex.unlock(lp.io); if (self.terminatePending()) return; v8.v8__Isolate__PerformMicrotaskCheckpoint(self.isolate.handle); - if (self.terminatePending()) { - // See runMicrotasks: a checkpoint exit doesn't clear V8's terminating - // state the way a JS entry unwind does. - clearTerminationState(self.isolate.handle); - } } fn promiseRejectCallback(message_handle: v8.PromiseRejectMessage) callconv(.c) void { diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index 325bb8381..4deb9336a 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -347,6 +347,130 @@ test "Function: requested termination is classified and blocks re-entry" { try testing.expectEqual(3, try (try local.exec("1 + 2", null)).toI32()); } +test "Function: nested microtask checkpoint keeps the caller's termination" { + const frame = try testing.createFrame(); + defer testing.test_session.closeAllPages(); + + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + const local = &ls.local; + + const env = frame.js.env; + defer env.cancelTerminate(); + + const State = struct { + env: *js.Env, + local: *const js.Local, + resumed: bool = false, + + fn kill(self: *@This()) void { + self.env.requestTerminate(); + } + + // Draining the queue from a native call runs at call depth >= 1, with + // the caller's JS still on the stack. A terminate landing in here is + // aimed at that caller too, so the checkpoint must not clear it. + fn pump(self: *@This()) void { + self.local.runMicrotasks(); + } + + fn markResumed(self: *@This()) void { + self.resumed = true; + } + }; + var state = State{ .env = env, .local = local }; + + const driver = try local.exec( + \\(function(kill, pump, resumed) { + \\ Promise.resolve().then(function(){ kill(); for(;;){} }); + \\ pump(); + \\ resumed(); + \\}) + , null); + const driver_fn = Function{ .local = local, .handle = @ptrCast(driver.handle) }; + + var caught: js.TryCatch.Caught = .{}; + const args = .{ + local.newCallback(State.kill, &state), + local.newCallback(State.pump, &state), + local.newCallback(State.markResumed, &state), + }; + try testing.expectError(error.ExecutionTerminated, driver_fn.tryCall(void, args, &caught)); + try testing.expectEqual(false, state.resumed); + try testing.expectEqual(true, env.terminatePending()); +} + +test "Function: a terminated checkpoint stops the context loop" { + const frame = try testing.createFrame(); + const other = try testing.createFrame(); + defer testing.test_session.closeAllPages(); + + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + const local = &ls.local; + + const env = frame.js.env; + defer env.cancelTerminate(); + + // The second context's queue is the one the loop must not go on to reach: + // entering a checkpoint on a terminating isolate consumes the termination, + // which would let the wedged caller below resume. + { + var other_ls: js.Local.Scope = undefined; + other.js.localScope(&other_ls); + defer other_ls.deinit(); + try other_ls.local.eval( + \\window.__ran = false; + \\Promise.resolve().then(function(){ window.__ran = true; }); + , null); + } + + const State = struct { + env: *js.Env, + local: *const js.Local, + resumed: bool = false, + + fn kill(self: *@This()) void { + self.env.requestTerminate(); + } + + fn pump(self: *@This()) void { + self.local.runMicrotasks(); + } + + fn markResumed(self: *@This()) void { + self.resumed = true; + } + }; + var state = State{ .env = env, .local = local }; + + const driver = try local.exec( + \\(function(kill, pump, resumed) { + \\ Promise.resolve().then(function(){ kill(); for(;;){} }); + \\ pump(); + \\ resumed(); + \\}) + , null); + const driver_fn = Function{ .local = local, .handle = @ptrCast(driver.handle) }; + + var caught: js.TryCatch.Caught = .{}; + const args = .{ + local.newCallback(State.kill, &state), + local.newCallback(State.pump, &state), + local.newCallback(State.markResumed, &state), + }; + try testing.expectError(error.ExecutionTerminated, driver_fn.tryCall(void, args, &caught)); + try testing.expectEqual(false, state.resumed); + + env.cancelTerminate(); + var other_ls: js.Local.Scope = undefined; + other.js.localScope(&other_ls); + defer other_ls.deinit(); + try testing.expectEqual(false, (try other_ls.local.exec("window.__ran", null)).toBool()); +} + // A cheap, copyable handle to a persisted function. See js.GlobalSlot. pub const Global = struct { slot: *js.GlobalSlot,