Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/jsc/JSValue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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;
}

// ──────────────────────────────────────────────────────────────────────────
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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
Expand Down
14 changes: 14 additions & 0 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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);
Expand Down
17 changes: 12 additions & 5 deletions src/jsc/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Comment thread
robobun marked this conversation as resolved.
#[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.
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1215,8 +1223,7 @@ impl EventLoop {
this_value: JSValue,
arguments: &[JSValue],
) -> JsResult<JSValue> {
// 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)?;
Expand Down
11 changes: 10 additions & 1 deletion src/jsc/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -316,6 +318,7 @@ impl<C: JobContext> Job<C> {
cancel: |p| unsafe { C::cancel(&raw mut (*p.cast::<Self>()).off) },
prev: core::ptr::null_mut(),
next: core::ptr::null_mut(),
generation: cx.vm().test_isolation_generation,
},
ticket: Some(cx.vm().ticket()),
task: WorkPoolTask {
Expand Down Expand Up @@ -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<C>`, 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(());
Expand Down
92 changes: 92 additions & 0 deletions test/cli/test/isolation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
robobun marked this conversation as resolved.
});

// 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:
Expand Down
Loading