diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 69ee81760214..db8cc66f2dec 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -148,6 +148,8 @@ export const setSyntheticAllocationLimitForTesting: (limit: number) => number = 1, ); +export const refStringsCount: () => number = $newRustFunction("virtual_machine_exports.rs", "Bun__refStringsCount", 0); + // Shrink the markdown parser's block-metadata cap (in bytes) so its // `TooManyBlocks` error is reachable without 4 GiB of input. The cap can only // be lowered, never raised past the real limit. Returns the previous value so diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 191c40190c98..2129293eb17e 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -3764,7 +3764,7 @@ impl VirtualMachine { specifier, source_url: create_if_different(&specifier, source_url), allocator: source.cast::(), - source_code_needs_deref: false, + source_code_needs_deref: true, ..Default::default() } } @@ -3832,7 +3832,7 @@ impl VirtualMachine { } } - /// Interns `input_` in the VM's ref-string map and returns the ref-counted entry. + /// Interns `input_` and returns the entry with exactly +1 owed to the caller. pub fn ref_counted_string( &mut self, input_: &[u8], @@ -3840,7 +3840,12 @@ impl VirtualMachine { ) -> *mut crate::ref_string::RefString { debug_assert!(!input_.is_empty()); let mut was_new = false; - self.ref_counted_string_with_was_new::(&mut was_new, input_, hash_) + let r = self.ref_counted_string_with_was_new::(&mut was_new, input_, hash_); + if !was_new { + // SAFETY: `r` is live in `self.ref_strings`; fresh entries already have +1 from `create_external`. + unsafe { (*r).ref_() }; + } + r } // Note: `flags` is a runtime arg — diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a45cf0016dfe..9dc498a12e4b 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -180,6 +180,7 @@ #include "EventLoopTask.h" #include "NodeModuleModule.h" #include +#include #include "JSPerformanceServerTiming.h" #include "JSPerformanceResourceTiming.h" #include "JSPerformanceTiming.h" @@ -3536,6 +3537,9 @@ void GlobalObject::reload() this->requireMap()->clear(this); RETURN_IF_EXCEPTION(scope, ); + // Stale entries (keyed by old source text) pin their old SourceProvider. + vm.codeCache()->clear(); + // If we run the GC every time, we will never get the SourceProvider cache hit. // So we run the GC every other time. if ((this->reloadCount++ + 1) % 2 == 0) { diff --git a/src/jsc/virtual_machine_exports.rs b/src/jsc/virtual_machine_exports.rs index 749067bd49ce..2dc5de3a3552 100644 --- a/src/jsc/virtual_machine_exports.rs +++ b/src/jsc/virtual_machine_exports.rs @@ -346,3 +346,10 @@ pub fn Bun__setSyntheticAllocationLimitForTesting( .store(limit, core::sync::atomic::Ordering::Relaxed); Ok(JSValue::js_number(prev as f64)) } + +/// Live entries in the VM's ref-string cache (native-heap; invisible to heapStats). +#[crate::host_fn(export = "Bun__refStringsCount")] +pub fn Bun__refStringsCount(_global: &JSGlobalObject, _frame: &CallFrame) -> JsResult { + let count = VirtualMachine::get().ref_strings.count(); + Ok(JSValue::js_number(f64::from(count))) +} diff --git a/src/runtime/api/filesystem_router.rs b/src/runtime/api/filesystem_router.rs index 3557270fa979..ea5fa3969bab 100644 --- a/src/runtime/api/filesystem_router.rs +++ b/src/runtime/api/filesystem_router.rs @@ -313,18 +313,9 @@ impl FileSystemRouter { root_dir_info.abs_path }; - // Note: `vm.refCountedString` is an interning cache — on a cache HIT it - // returns the existing `*mut RefString` WITHOUT bumping the refcount. - // `getScriptSrc`/`getOrigin` use `.leak()` (no ref), so without an - // explicit hold N routers sharing one interned RefString → N finalizers - // deref a single +1 → UAF on the second deref. Claim an explicit +1 here - // so each `FileSystemRouter` owns its hold; `finalize` releases it. + // `ref_counted_string` hands back +1; `finalize` releases it. let claim = |p: *mut RefString| -> BackRef { - // `ref_counted_string` returns a live interned `*mut RefString`; wrap as - // `BackRef` (owner-outlives-holder: VM intern cache + our +1). - let r = BackRef::from(core::ptr::NonNull::new(p).expect("ref_counted_string")); - r.ref_(); - r + BackRef::from(core::ptr::NonNull::new(p).expect("ref_counted_string")) }; let fs_router = Box::new(FileSystemRouter { origin: if !origin_str.slice().is_empty() { diff --git a/src/runtime/dispatch_js2native.rs b/src/runtime/dispatch_js2native.rs index 500fe8b5bbae..66487547333f 100644 --- a/src/runtime/dispatch_js2native.rs +++ b/src/runtime/dispatch_js2native.rs @@ -39,6 +39,7 @@ pub use bun_install_jsc::ini_jsc::ini_testing_parse as ini_ini_ini_testing_ap_is pub use bun_jsc::bindgen_test::get_bindgen_test_functions as jsc_bindgen_test_get_bindgen_test_functions; pub use bun_jsc::counters::create_counters_object as jsc_counters_create_counters_object; pub use bun_jsc::event_loop::get_active_tasks as jsc_event_loop_get_active_tasks; +pub use bun_jsc::virtual_machine_exports::Bun__refStringsCount as jsc_virtual_machine_exports_bun__ref_strings_count; pub use bun_jsc::virtual_machine_exports::Bun__setSyntheticAllocationLimitForTesting as jsc_virtual_machine_exports_bun__set_synthetic_allocation_limit_for_testing; // `emit_handle_ipc_message` is implemented in this crate (`ipc_host.rs`) // because it dereferences `Subprocess`, a runtime type. diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 832e61e79cc8..a215196b5c06 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -772,7 +772,6 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error(${counter});`, expect(reloadCounter).toBe(50); bundler.kill(); await runner.exited; - // TODO: bun has a memory leak when --hot is used on very large files }, longTimeout, ); diff --git a/test/regression/issue/11083.test.ts b/test/regression/issue/11083.test.ts new file mode 100644 index 000000000000..cd34bc715480 --- /dev/null +++ b/test/regression/issue/11083.test.ts @@ -0,0 +1,83 @@ +// https://github.com/oven-sh/bun/issues/11083 + +import { spawn } from "bun"; +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isDebug, tempDir } from "harness"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; + +test( + "bun --hot should not accumulate stale code blocks or ref-string entries when file content changes on every reload", + async () => { + using dir = tempDir("hot-11083", {}); + const root = join(String(dir), "leak-runner.mjs"); + + // Small file so debug builds cycle faster than JSC's ~10s CodeCache prune + // timer; unique content each iteration so the cache key never repeats. + const writeSource = (iter: number) => { + writeFileSync( + root, + `var unused_${iter} = ${iter}; +Bun.gc(true); +globalThis.__i = (globalThis.__i ?? 0) + 1; +const s = require("bun:jsc").heapStats(); +const { refStringsCount } = require("bun:internal-for-testing"); +console.error(JSON.stringify({ + i: globalThis.__i, + umpcb: s.objectTypeCounts.UnlinkedModuleProgramCodeBlock || 0, + refs: refStringsCount(), +})); +`, + ); + }; + + let iter = 0; + writeSource(++iter); + + await using runner = spawn({ + cmd: [bunExe(), "--hot", "run", root], + env: bunEnv, + cwd: String(dir), + stdout: "ignore", + stderr: "pipe", + stdin: "ignore", + }); + + const target = 50; + let maxCodeBlocks = 0; + let maxRefStrings = 0; + let reached = 0; + let buf = ""; + outer: for await (const chunk of runner.stderr!) { + buf += new TextDecoder().decode(chunk); + let nl: number; + let progressed = false; + while ((nl = buf.indexOf("\n")) >= 0) { + const line = buf.slice(0, nl); + buf = buf.slice(nl + 1); + if (!line.startsWith("{")) { + if (/Watcher crashed|panic:|oh no:|TypeError:|is not a function/.test(line)) { + throw new Error("child --hot died: " + line); + } + continue; + } + const { i, umpcb, refs } = JSON.parse(line); + reached = i; + maxCodeBlocks = Math.max(maxCodeBlocks, umpcb); + maxRefStrings = Math.max(maxRefStrings, refs); + progressed = true; + if (i >= target) { + runner.kill(); + break outer; + } + } + if (progressed) writeSource(++iter); + } + + expect(reached).toBe(target); + expect(maxCodeBlocks).toBeLessThan(10); + expect(maxRefStrings).toBeGreaterThan(0); + expect(maxRefStrings).toBeLessThan(10); + }, + isDebug ? 60_000 : 20_000, +);