diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index 09e31b639806..e9852689e0a6 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -352,6 +352,11 @@ impl JSValue { } JSC__JSValue__isAnyError(self) } + /// Whether this object's realm was retired by `bun test --isolate` (its file finished). + #[inline] + pub fn is_from_retired_test_isolation_realm(self) -> bool { + self.is_cell() && Bun__JSValue__isFromRetiredTestIsolationRealm(self) + } /// `JSValue.isError()` — true iff this is an /// `ErrorInstance` cell (does NOT match `Exception`). #[inline] @@ -2136,6 +2141,7 @@ unsafe extern "C" { ) -> JSValue; safe fn Bun__JSValue__protect(this: JSValue); safe fn Bun__JSValue__unprotect(this: JSValue); + safe fn Bun__JSValue__isFromRetiredTestIsolationRealm(this: JSValue) -> bool; } // ────────────────────────────────────────────────────────────────────────── diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 962044c4b021..f2ef9dc528aa 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -411,6 +411,7 @@ unsafe extern "C" { safe fn Zig__GlobalObject__prepareForDestruction(global: &JSGlobalObject); safe fn Zig__GlobalObject__forbidExecution(global: &JSGlobalObject); safe fn Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(global: &JSGlobalObject); + safe fn Zig__GlobalObject__retireForTestIsolation(global: &JSGlobalObject); safe fn Zig__GlobalObject__destructOnExit(global: &JSGlobalObject); safe fn WebWorker__teardownJSCVM(global: &JSGlobalObject); } @@ -5177,6 +5178,9 @@ impl VirtualMachine { let _ = self.auto_killer.kill(); self.auto_killer.clear(); + // The outgoing file's exit: work it left in flight (thread-pool jobs, + // the children just killed) lands later and must not resume its script. + Zig__GlobalObject__retireForTestIsolation(self.global()); self.test_isolation_generation = self.test_isolation_generation.wrapping_add(1); // Generation-stale JS timers would otherwise release their pins only diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 1bcd27a1a078..b5921f3281de 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4380,6 +4380,20 @@ extern "C" void Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(Zig::Glo globalObject->scriptExecutionContext()->prepareForDestruction(); } +// `bun test --isolate`: JSC drops microtasks queued against the finished file's realm from here on +// (as WebCore does for a stopped document), so work it left in flight cannot resume its script. +extern "C" void Zig__GlobalObject__retireForTestIsolation(Zig::GlobalObject* globalObject) +{ + globalObject->setMicrotaskRunnability(JSC::QueuedTaskResult::Discard); +} + +// Whether `value` is an object of a realm retired above; native code does not call into one. +extern "C" bool Bun__JSValue__isFromRetiredTestIsolationRealm(JSC::EncodedJSValue encodedValue) +{ + JSC::JSObject* object = JSC::JSValue::decode(encodedValue).getObject(); + return object && object->globalObject()->microtaskRunnability() == JSC::QueuedTaskResult::Discard; +} + extern "C" void Zig__GlobalObject__destructOnExit(Zig::GlobalObject* globalObject) { auto& vm = JSC::getVM(globalObject); diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index f3c3b0804675..e63b3583a000 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -460,6 +460,15 @@ impl EventLoop { Ok(()) } + /// `run_callback*`'s gate; also refuses a function of a realm that + /// `bun test --isolate` retired (a killed child's late `onExit`). + #[inline] + fn may_enter_js(callback: JSValue, global_object: &JSGlobalObject) -> bool { + !global_object.has_exception() + && !(global_object.bun_vm().test_isolation_enabled + && callback.is_from_retired_test_isolation_realm()) + } + /// When you call a JavaScript function from outside the event loop task /// queue, it has to be wrapped in `runCallback` to ensure that microtasks /// are drained and errors are handled. @@ -476,7 +485,7 @@ impl EventLoop { // exception already pending — a prior callback's microtasks can request // termination (worker.terminate()), and entering JS then would trip // executeCallImpl's `assertNoException`. - if global_object.has_exception() { + if !Self::may_enter_js(callback, global_object) { return; } // R-2 noalias mitigation (see PORT_NOTES_PLAN R-2; precedent @@ -513,8 +522,7 @@ impl EventLoop { this_value: JSValue, arguments: &[JSValue], ) -> JSValue { - // Same gate as `run_callback`. - if global_object.has_exception() { + if !Self::may_enter_js(callback, global_object) { return JSValue::ZERO; } // R-2 noalias mitigation — see `run_callback` above. @@ -1215,8 +1223,7 @@ impl EventLoop { this_value: JSValue, arguments: &[JSValue], ) -> JsResult { - // Same gate as `run_callback`. - if global_object.has_exception() { + if !Self::may_enter_js(callback, global_object) { return Ok(JSValue::UNDEFINED); } let result = callback.call(global_object, this_value, arguments)?; diff --git a/src/jsc/job.rs b/src/jsc/job.rs index 55c2c6716487..964f6eddfe70 100644 --- a/src/jsc/job.rs +++ b/src/jsc/job.rs @@ -223,6 +223,8 @@ pub struct JobHeader { cancel: unsafe fn(*mut JobHeader), prev: *mut JobHeader, next: *mut JobHeader, + /// `VirtualMachine::test_isolation_generation` when scheduled. + generation: u32, } /// A VM's live [cancellable](JobContext::CANCELLABLE) jobs (JS thread only; @@ -316,6 +318,7 @@ impl Job { cancel: |p| unsafe { C::cancel(&raw mut (*p.cast::()).off) }, prev: core::ptr::null_mut(), next: core::ptr::null_mut(), + generation: cx.vm().test_isolation_generation, }, ticket: Some(cx.vm().ticket()), task: WorkPoolTask { @@ -433,7 +436,13 @@ pub unsafe fn complete_erased(ptr: *mut (), cx: &JsThread<'_>) -> JsResult<()> { // build script-facing values under a pending termination. Release it as // teardown would — Node's threadpool `after` callbacks bail the same way // on `!can_call_into_js()`. - if !cx.vm().script_allowed() { + // + // Likewise one scheduled by a file `bun test --isolate` has since retired: + // the swap was that file's exit, and a `then` that calls back directly + // (node:crypto's callback forms) would run its script under the next file. + // SAFETY: `ptr` is a live posted `Job`, header first (fn contract). + let stale = unsafe { (*header).generation } != cx.vm().test_isolation_generation; + if !cx.vm().script_allowed() || stale { // SAFETY: as below; released exactly once, here. unsafe { ((*header).release_unrun)(header) }; return Ok(()); diff --git a/test/cli/test/isolation.test.ts b/test/cli/test/isolation.test.ts index 7678f5d99b7c..9168eed72d1f 100644 --- a/test/cli/test/isolation.test.ts +++ b/test/cli/test/isolation.test.ts @@ -1077,6 +1077,98 @@ test.concurrent("--isolate: leaked AbortSignal.timeout does not fire in next fil expect(exitCode).toBe(0); }); +// The swap sweeps what exists AT the file boundary. Work a finished file left +// in flight lands later, while the next file runs: a thread-pool job settles a +// promise of the retired realm, a child the swap killed reports its exit. The +// finished file's continuation must not run then. Before the fence it did, and +// whatever it created (a setInterval, a Bun.serve, a child process, a chdir) +// was adopted by the running file. +describe.concurrent("--isolate: a finished file's late completions do not run in the next file", () => { + const lateFixtures = { + "a-late.test.ts": ` + import { test, expect } from "bun:test"; + import { existsSync, writeFileSync } from "node:fs"; + import { tmpdir } from "node:os"; + import { join } from "node:path"; + + const dir = import.meta.dir; + + test("leaks a thread-pool chain and a child with onExit", () => { + // Each turn hops through the thread pool, so the chain is always mid-flight + // at the swap. It only acts once b-late.test.ts says it is running. + (async () => { + while (!existsSync(join(dir, "b-running"))) { + await Bun.password.hash("pw", { algorithm: "bcrypt", cost: 4 }); + } + process.chdir(tmpdir()); + const server = Bun.serve({ port: 0, fetch: () => new Response("served by dead A") }); + writeFileSync(join(dir, "a-acted"), String(server.port)); + })(); + + // Killed by the swap; its exit lands while B runs. + Bun.spawn({ + cmd: [process.execPath, "-e", "setInterval(() => {}, 1000)"], + stdio: ["ignore", "ignore", "ignore"], + onExit() { + writeFileSync(join(dir, "a-onexit"), ""); + }, + }); + + expect(existsSync(join(dir, "b-running"))).toBe(false); + }); + `, + "b-late.test.ts": ` + import { test, expect } from "bun:test"; + import { existsSync, realpathSync, writeFileSync } from "node:fs"; + import { join } from "node:path"; + + const dir = import.meta.dir; + + test("sees nothing from A", async () => { + writeFileSync(join(dir, "b-running"), ""); + // Turn the event loop through the thread pool so A's pending job (and + // its killed child's exit) get every chance to land here. + for (let i = 0; i < 40 && !existsSync(join(dir, "a-acted")); i++) { + await Bun.password.hash("pw", { algorithm: "bcrypt", cost: 4 }); + } + expect({ + acted: existsSync(join(dir, "a-acted")), + onExit: existsSync(join(dir, "a-onexit")), + cwd: realpathSync("."), + }).toEqual({ + acted: false, + onExit: false, + cwd: realpathSync(dir), + }); + }); + `, + }; + const files = ["./a-late.test.ts", "./b-late.test.ts"]; + + test("--isolate", async () => { + using dir = tempDir("isolate-late", lateFixtures); + const { stderr, exitCode } = await runTests(String(dir), ["--isolate"], files); + expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass"); + expect(normalizeBunSnapshot(stderr, dir)).toContain("0 fail"); + if (exitCode !== 0) expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // One worker takes both files (scale-up gated), so the same fence applies + // between files inside a --parallel worker. + test("--parallel", async () => { + using dir = tempDir("isolate-late-parallel", lateFixtures); + const { stderr, exitCode } = await runTests(String(dir), ["--parallel=2"], files, { + ...bunEnv, + BUN_TEST_PARALLEL_SCALE_MS: "60000", + }); + expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass"); + expect(normalizeBunSnapshot(stderr, dir)).toContain("0 fail"); + if (exitCode !== 0) expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); + // Each of these leaked handles used to pin its test file's ENTIRE global // object (and therefore the file's module graph) for the rest of a // `bun test --isolate` run, growing memory by one full global per file: