diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index 092d29a0f87a..e1234a1c32d4 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -29,6 +29,7 @@ unsafe extern "C" { safe fn JSC__VM__runGC(vm: &VM, sync: bool) -> usize; safe fn JSC__VM__heapSize(vm: &VM) -> usize; safe fn JSC__VM__collectAsync(vm: &VM); + safe fn JSC__VM__collectFullAsync(vm: &VM); safe fn JSC__VM__executionForbidden(vm: &VM) -> bool; safe fn JSC__VM__notifyNeedTermination(vm: &VM); safe fn JSC__VM__isEntered(vm: &VM) -> bool; @@ -98,6 +99,11 @@ impl VM { JSC__VM__collectAsync(self) } + /// Non-blocking full collection on the GC thread (unlike [`VM::run_gc`]). + pub(crate) fn collect_full_async(&self) { + JSC__VM__collectFullAsync(self) + } + pub fn execution_forbidden(&self) -> bool { JSC__VM__executionForbidden(self) } diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index f24fecbb6a49..702871c3a6b8 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -5114,12 +5114,16 @@ impl VirtualMachine { self.unhandled_error_counter = 0; let old_global = self.global; - // `old_global` valid for VM lifetime (safe ZST-handle deref); - // `console` is the live per-VM ConsoleObject. - let new_global: *mut JSGlobalObject = JSGlobalObject::create_for_test_isolation( - JSGlobalObject::opaque_ref(old_global), - self.console.cast(), - ); + // Hold a Rust-side scope across the FFI call so the C++ ThrowScope's exception check is balanced. + let new_global: *mut JSGlobalObject = { + crate::top_scope!(scope, self.global()); + let new_global = JSGlobalObject::create_for_test_isolation( + JSGlobalObject::opaque_ref(old_global), + self.console.cast(), + ); + let _ = scope.assert_no_exception_except_termination(); + new_global + }; self.global = new_global; VMHolder::set_cached_global_object(Some(new_global)); self.regular_event_loop.global = NonNull::new(new_global); @@ -5131,6 +5135,9 @@ impl VirtualMachine { } } } + + // Reclaim the old file's now-detached module graph (the per-file loop has no GC pressure). + self.global().vm().collect_full_async(); } /// Loads and evaluates a macro entry module, waiting for its promise. diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index b2f8ffd52eed..6a7bfc4afd1f 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -700,9 +700,14 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__createForTestIsolation(Zig::G globalObject->m_processEnvObject.set(vm, globalObject, Bun::createSharedEnvironmentVariablesMap(globalObject).getObject()); } - // Drop the permanent root on the previous global so its module registry, - // require.cache, and user objects become collectable. JSC's CodeCache and - // Bun's RuntimeTranspilerCache are VM/process scoped and survive. + // Detach the outgoing file's module graph so it's reclaimed even if the old global lingers. + { + auto scope = DECLARE_THROW_SCOPE(vm); + oldGlobal->clearModuleRegistry(); + scope.assertNoException(); + } + + // Drop the permanent root on the previous global so it becomes collectable. oldGlobal->isThreadLocalDefaultGlobalObject = false; JSC::gcUnprotect(oldGlobal); @@ -3578,16 +3583,22 @@ template void GlobalObject::visitOutputConstraints(JSCell*, SlotVisitor&); // DEFINE_VISIT_CHILDREN(Zig::GlobalObject); -void GlobalObject::reload() +void GlobalObject::clearModuleRegistry() { - auto& vm = this->vm(); - auto scope = DECLARE_THROW_SCOPE(vm); + auto* moduleLoader = this->moduleLoader(); { - auto* moduleLoader = this->moduleLoader(); + // cellLock() pairs with the GC thread's visitChildrenImpl over these maps. WTF::Locker locker { moduleLoader->cellLock() }; moduleLoader->clearAll(); } this->requireMap()->clear(this); +} + +void GlobalObject::reload() +{ + auto& vm = this->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + this->clearModuleRegistry(); RETURN_IF_EXCEPTION(scope, ); // If we run the GC every time, we will never get the SourceProvider cache hit. @@ -4296,17 +4307,10 @@ void GlobalObject::forbidExecution() // MicrotaskQueue references Heap. vm.defaultMicrotaskQueue().clear(); - // Drop the module registry and require() cache so module-level bindings become unreachable - // for the final collection (their ExternalStringImpl deallocators must run before ~VM). - { - auto* moduleLoader = this->moduleLoader(); - // JSModuleLoader::visitChildrenImpl iterates these maps on the GC thread under cellLock(). - WTF::Locker locker { moduleLoader->cellLock() }; - moduleLoader->clearAll(); - } + // Drop module registry + require cache so module bindings die before ~VM. { auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); - requireMap()->clear(this); + this->clearModuleRegistry(); scope.clearException(); } diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 09559a1e4b5a..748ef5bd5a9d 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -769,6 +769,9 @@ class GlobalObject : public Bun::GlobalScope { void reload(); + // Drop the ESM module registry and CJS require cache so module state becomes collectable. + void clearModuleRegistry(); + JSC::Structure* jsonlParseResultStructure() { return m_jsonlParseResultStructure.get(this); } JSC::Structure* pathParsedObjectStructure() { return m_pathParsedObjectStructure.get(this); } JSC::Structure* pendingVirtualModuleResultStructure() { return m_pendingVirtualModuleResultStructure.get(this); } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 87b704a4b042..e7b62715e380 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3046,6 +3046,13 @@ void JSC__VM__collectAsync(JSC::VM* vm) vm->heap.collectAsync(); } +// Non-blocking full (not eden) collection on the GC thread (unlike JSC__VM__runGC's collectNow). +void JSC__VM__collectFullAsync(JSC::VM* vm) +{ + JSC::JSLockHolder lock(*vm); + vm->heap.collectAsync(JSC::GCRequest(JSC::CollectionScope::Full)); +} + size_t JSC__VM__heapSize(JSC::VM* arg0) { return arg0->heap.size(); diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index 07b00db396a0..ce2a057f704d 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -298,6 +298,7 @@ CPP_DECL void JSC__JSValue__toZigString(JSC::EncodedJSValue JSValue0, ZigString* CPP_DECL size_t JSC__VM__blockBytesAllocated(JSC::VM* arg0); CPP_DECL void JSC__VM__collectAsync(JSC::VM* arg0); +CPP_DECL void JSC__VM__collectFullAsync(JSC::VM* arg0); CPP_DECL JSC::VM* JSC__VM__create(unsigned char HeapType0); CPP_DECL void JSC__VM__deleteAllCode(JSC::VM* arg0, JSC::JSGlobalObject* arg1); CPP_DECL void JSC__VM__drainMicrotasks(JSC::VM* arg0); diff --git a/src/runtime/cli/test/parallel/runner.rs b/src/runtime/cli/test/parallel/runner.rs index 7430871524cc..4301b8eae441 100644 --- a/src/runtime/cli/test/parallel/runner.rs +++ b/src/runtime/cli/test/parallel/runner.rs @@ -573,6 +573,7 @@ impl<'a> WorkerLoop<'a> { ) { test_command::handle_top_level_test_error_before_javascript_start(&err); } + Global::mimalloc_cleanup(false); if vm.test_isolation_enabled { crate::jsc_hooks::stop_active_handles_for_test_isolation(vm); vm.swap_global_for_test_isolation(); @@ -580,8 +581,6 @@ impl<'a> WorkerLoop<'a> { .jest .bun_test_root .reset_hook_scope_for_test_isolation(); - } else { - Global::mimalloc_cleanup(false); } self.reporter.jest.default_timeout_override = u32::MAX; diff --git a/test/regression/issue/31771.test.ts b/test/regression/issue/31771.test.ts new file mode 100644 index 000000000000..4145599f59ce --- /dev/null +++ b/test/regression/issue/31771.test.ts @@ -0,0 +1,111 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isLinux, tempDir } from "harness"; +import fs from "node:fs"; +import path from "node:path"; + +// https://github.com/oven-sh/bun/issues/31771 +// +// `bun test --isolate` runs each file in a fresh global within ONE process. +// The module records / namespace objects instantiated for one file must be +// reclaimed when the global is swapped for the next file. Before the fix the +// outgoing global's module registry and require cache were never cleared, so +// each finished file's module graph was retained and peak RSS grew linearly +// with the number of files (proportional to each file's imported graph size), +// OOMing large suites. After the fix peak RSS stays roughly flat, as it does +// in default (shared-global) mode. +// +// Measured by polling VmHWM (peak RSS high-water mark, monotonic) of the child +// from /proc. Serial --isolate is a single process, so this is exact. The +// signal is the SLOPE (RSS grows with file count), not an absolute number, so +// the threshold is robust across platforms and build types. + +// A 500-module import graph shared by every test file. Before the fix this +// retained several MB/file; the difference between a small and large file +// count is tens of MB — far above allocator noise. Kept modest so the run +// completes well within the timeout under ASAN (the debug/gate build). +const MODULE_COUNT = 500; +const SMALL_N = 4; +const LARGE_N = 10; + +function makeFixtures() { + const files: Record = {}; + let graph = ""; + for (let i = 0; i < MODULE_COUNT; i++) { + files[`mods/m${i}.ts`] = `export function f${i}() { return ${i}; }\n`; + graph += `import "./mods/m${i}";\n`; + } + files["graph.ts"] = graph; + for (let i = 0; i < LARGE_N; i++) { + files[`t${i}.test.ts`] = `import "./graph";\nimport { test } from "bun:test";\ntest("noop", () => {});\n`; + } + return files; +} + +async function peakRssMb(dir: string, n: number): Promise { + const files = Array.from({ length: n }, (_, i) => `./t${i}.test.ts`); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--isolate", ...files], + env: bunEnv, + cwd: dir, + stdout: "ignore", + stderr: "pipe", + }); + + let peakKb = 0; + const statusPath = `/proc/${proc.pid}/status`; + let running = true; + const poll = (async () => { + while (running) { + try { + const status = fs.readFileSync(statusPath, "utf8"); + const m = status.match(/VmHWM:\s*(\d+)\s*kB/); + if (m) peakKb = Math.max(peakKb, parseInt(m[1], 10)); + } catch { + // process gone + } + await Bun.sleep(2); + } + })(); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + running = false; + await poll; + + // Combined assertion so a failed/killed child surfaces its stderr tail, + // exit code, and signal together in one diff. + const summaryOk = stderr.includes(`${n} pass`) && stderr.includes("0 fail"); + expect({ + summary: summaryOk ? "ok" : stderr.slice(-2000), + exitCode, + signalCode: proc.signalCode, + sawPeakRss: peakKb > 0, + }).toEqual({ summary: "ok", exitCode: 0, signalCode: null, sawPeakRss: true }); + return peakKb / 1024; +} + +test.skipIf(!isLinux)( + "bun test --isolate does not retain finished files' module graph (peak RSS stays flat)", + async () => { + using dir = tempDir("isolate-rss-31771", makeFixtures()); + const root = String(dir); + + // Sanity: fixtures materialized. + expect(fs.existsSync(path.join(root, "graph.ts"))).toBe(true); + + const small = await peakRssMb(root, SMALL_N); + const large = await peakRssMb(root, LARGE_N); + + const perFile = (large - small) / (LARGE_N - SMALL_N); + + // Before the fix the 500-module graph is retained per file, so the slope is + // several MB/file (tens of MB across the file-count delta). After the fix + // the graph is reclaimed on each swap and the slope is near zero. 0.5 MB/file + // sits comfortably between the two (post-fix is <0.1 MB/file even under + // ASAN's noisier heap accounting). + expect(perFile).toBeLessThan(0.5); + }, + // Outlier per root CLAUDE.md: spawns two child `bun test --isolate` runs that + // each transpile a 500-module graph under debug+ASAN (~14s total), which no + // module count small enough to fit the default budget would still discriminate. + 30_000, +);