diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index bc10d428e5b2..1ac51c9bf050 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -907,9 +907,9 @@ pub struct MarkedArrayBuffer { pub owns_buffer: bool, } -/// Bytes produced off-thread (`from_bytes`/`from_string`) are owned until they -/// are handed to JSC; a result that is never converted (its VM went away, the -/// conversion path bailed) frees them here. +/// Bytes produced off-thread (`from_owned_bytes`/`from_string`) are owned until +/// they are handed to JSC; a result that is never converted (its VM went away, +/// the conversion path bailed) frees them here. impl Drop for MarkedArrayBuffer { fn drop(&mut self) { self.destroy(); @@ -932,15 +932,13 @@ impl MarkedArrayBuffer { } pub fn from_string(str: &[u8]) -> Result { - // allocator.dupe(u8, str) → Box::<[u8]>::from(str), but we need a raw - // pointer because the buffer is later freed via the default allocator - // (`MarkedArrayBuffer_deallocator` → `default_alloc::free`). - let buf: Box<[u8]> = Box::from(str); - let len = buf.len(); - let ptr = bun_core::heap::into_raw(buf).cast::(); - // SAFETY: ptr/len from heap::alloc; backed by the global allocator. - let bytes = unsafe { bun_core::ffi::slice_mut(ptr, len) }; - Ok(MarkedArrayBuffer::from_bytes(bytes, JSType::Uint8Array)) + // allocator.dupe(u8, str) → Box::<[u8]>::from(str); the buffer is later + // freed via the default allocator (`destroy` or + // `MarkedArrayBuffer_deallocator` → `default_alloc::free`). + Ok(MarkedArrayBuffer::from_owned_bytes( + Box::from(str), + JSType::Uint8Array, + )) } pub fn from_js(global: &JSGlobalObject, value: JSValue) -> Option { @@ -951,12 +949,31 @@ impl MarkedArrayBuffer { }) } - pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> MarkedArrayBuffer { + /// Take ownership of a default-allocator `Box<[u8]>`. The bytes are freed + /// exactly once: by [`MarkedArrayBuffer::destroy`] (also run by `Drop`) if + /// the value is never converted, or by the deallocator JSC installs when + /// `to_node_buffer` hands them over. + /// + /// Requiring `Box<[u8]>` makes the ownership transfer a type-system + /// invariant. There is deliberately no constructor that adopts a borrowed + /// slice as owned storage: the former `from_bytes(&mut [u8], _)` let safe + /// code free a stack buffer (issue #31969). This must not compile + /// (checked by `cargo test --doc -p bun_jsc`): + /// + /// ```compile_fail,E0599 + /// use bun_jsc::{JSType, MarkedArrayBuffer}; + /// + /// let mut bytes = [0u8; 1]; + /// let buffer = MarkedArrayBuffer::from_bytes(&mut bytes, JSType::Uint8Array); + /// drop(buffer); // would free the stack address + /// ``` + pub fn from_owned_bytes(bytes: Box<[u8]>, typed_array_type: JSType) -> MarkedArrayBuffer { + // An empty boxed slice has no backing allocation (dangling ptr): + // nothing to own, so `destroy()` must not free it. + let owns_buffer = !bytes.is_empty(); MarkedArrayBuffer { - buffer: ArrayBuffer::from_bytes(bytes, typed_array_type), - // An empty boxed slice has no backing allocation (dangling ptr): - // nothing to own, so `destroy()` must not free it. - owns_buffer: !bytes.is_empty(), + buffer: ArrayBuffer::from_owned_bytes(bytes, typed_array_type), + owns_buffer, } } @@ -971,23 +988,28 @@ impl MarkedArrayBuffer { } /// Releases the owned byte buffer if this `MarkedArrayBuffer` was created with an - /// allocator (e.g. via `from_string`/`from_bytes`) and never handed to JSC. + /// allocator (e.g. via `from_string`/`from_owned_bytes`) and never handed to JSC. /// Idempotent; also what `Drop` does. pub fn destroy(&mut self) { if self.owns_buffer { self.owns_buffer = false; - // SAFETY: buffer.ptr was allocated by the global allocator (heap::alloc / allocator.dupe). + // SAFETY: `owns_buffer` is only set by `from_owned_bytes`, which took + // the pointer from a non-empty default-allocator `Box<[u8]>`. unsafe { bun_alloc::default_alloc::free(self.buffer.ptr.cast()) }; + // Neutralize the handle so a later `slice()` or handoff cannot + // observe the freed pointer. + self.buffer = ArrayBuffer::EMPTY; } } /// Ownership of the bytes moves to JSC (freed by the buffer's deallocator). pub fn to_node_buffer(&mut self, global: &JSGlobalObject) -> JsResult { - // `JSValue::create_buffer` takes `&mut [u8]` (ownership transfers to JSC - // via the deallocator). `ArrayBuffer` is `Copy` over a raw pointer, so - // copy the descriptor and project a mutable slice. + // Take the buffer out of `self` so neither `destroy()`/`Drop` nor a + // repeated handoff can release the allocation a second time. + // `JSValue::create_buffer` takes `&mut [u8]` and installs the + // deallocator over it. self.owns_buffer = false; - let mut buf = self.buffer; + let mut buf = core::mem::replace(&mut self.buffer, ArrayBuffer::EMPTY); JSValue::create_buffer(global, buf.byte_slice_mut()) } } diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index 1f8349d591cc..95c1a935a548 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -1903,14 +1903,15 @@ impl Terminal { return true; } v.extend_from_slice(chunk); - // MarkedArrayBuffer::from_bytes takes a `&mut [u8]` it will own (freed - // via mimalloc on the C++ side) — leak the Box and hand over the slice. - let bytes: &'static mut [u8] = Box::leak(v.into_boxed_slice()); - // This is the pipe reader's landing frame: a buffer that cannot be - // built (allocation failure, a terminating VM) is folded here and - // reading goes on. - let data = match MarkedArrayBuffer::from_bytes(bytes, jsc::JSType::Uint8Array) - .to_node_buffer(global_this) + // Ownership of the boxed slice transfers to JSC (freed via the + // buffer's deallocator). This is the pipe reader's landing frame: a + // buffer that cannot be built (allocation failure, a terminating VM) + // is folded here and reading goes on. + let data = match MarkedArrayBuffer::from_owned_bytes( + v.into_boxed_slice(), + jsc::JSType::Uint8Array, + ) + .to_node_buffer(global_this) { Ok(data) => data, Err(err) => { diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 828858d06994..d79280e0d919 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -6775,16 +6775,10 @@ impl NodeFS { if let Some(file) = graph.find_ref(path.as_bytes()) { let contents: &[u8] = file.utf8_contents(); return if args.encoding == Encoding::Buffer { - // PORTING.md §Forbidden bans `Vec::leak()`; round-trip through - // `into_boxed_slice()` so the allocation layout JSC frees with - // matches what we hand it (capacity == len). - let raw = - bun_core::heap::into_raw(contents.to_vec().into_boxed_slice()); - // SAFETY: ownership of the allocation is transferred to JSC; the - // ArrayBuffer finalizer reconstructs the Box and frees it - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, + // Ownership of the boxed slice transfers to the + // `Buffer` (freed by it, or by JSC once converted). + Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes( + contents.to_vec().into_boxed_slice(), bun_jsc::JSType::Uint8Array, ))) } else if string_type == ReadFileStringType::Default { @@ -6930,15 +6924,12 @@ impl NodeFS { }; } } - let raw = bun_core::heap::into_raw( + // Ownership of the boxed slice transfers to the `Buffer` + // (freed by it, or by JSC once converted). + Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes( temporary_read_buffer_before_stat_call .to_vec() .into_boxed_slice(), - ); - // SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, bun_jsc::JSType::Uint8Array, ))) } @@ -7097,11 +7088,10 @@ impl NodeFS { match args.encoding { Encoding::Buffer => { buf.truncate(final_len); - let raw = bun_core::heap::into_raw(buf.into_boxed_slice()); - // SAFETY: ownership transferred to JSC; freed via ArrayBuffer finalizer - // (PORTING.md:348 — `heap::alloc`/`from_raw` across FFI). - Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_bytes( - unsafe { &mut *raw }, + // Ownership of the boxed slice transfers to the `Buffer` + // (freed by it, or by JSC once converted). + Ok(ret::ReadFileWithOptions::Buffer(Buffer::from_owned_bytes( + buf.into_boxed_slice(), bun_jsc::JSType::Uint8Array, ))) } diff --git a/test/cli/run/cjs-fixture-leak-small.js b/test/cli/run/cjs-fixture-leak-small.js index 0e3a23d759a4..4cdd87d93583 100644 --- a/test/cli/run/cjs-fixture-leak-small.js +++ b/test/cli/run/cjs-fixture-leak-small.js @@ -1,7 +1,15 @@ const dest = require.resolve("./leak-fixture-small-ast.js"); // ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the threshold to avoid false positives. -const isASAN = process.execPath.includes("bun-asan"); +// run far higher under ASAN; widen the threshold to avoid false positives. +// Ask the runtime (same as harness.isASAN): debug builds also enable ASAN but +// are named bun-debug, so the executable name alone is not enough. +const isASAN = (() => { + try { + const { isASANEnabled } = require("bun:internal-for-testing"); + if (typeof isASANEnabled === "function") return isASANEnabled(); + } catch {} + return process.execPath.includes("bun-asan"); +})(); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint diff --git a/test/cli/run/esm-bug-leak-fixture.mjs b/test/cli/run/esm-bug-leak-fixture.mjs index 9884aa394222..789f77f8656c 100644 --- a/test/cli/run/esm-bug-leak-fixture.mjs +++ b/test/cli/run/esm-bug-leak-fixture.mjs @@ -2,8 +2,16 @@ import { createRequire } from "node:module"; const require = createRequire(import.meta.url); const dest = await import.meta.resolve("./esm-leak-fixture-large-ast.mjs"); // ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the threshold to avoid false positives. -const isASAN = process.execPath.includes("bun-asan"); +// run far higher under ASAN; widen the threshold to avoid false positives. +// Ask the runtime (same as harness.isASAN): debug builds also enable ASAN but +// are named bun-debug, so the executable name alone is not enough. +const isASAN = (() => { + try { + const { isASANEnabled } = require("bun:internal-for-testing"); + if (typeof isASANEnabled === "function") return isASANEnabled(); + } catch {} + return process.execPath.includes("bun-asan"); +})(); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint diff --git a/test/cli/run/esm-fixture-leak-small.mjs b/test/cli/run/esm-fixture-leak-small.mjs index 71fb83e6fa07..02423b4fcdf7 100644 --- a/test/cli/run/esm-fixture-leak-small.mjs +++ b/test/cli/run/esm-fixture-leak-small.mjs @@ -2,8 +2,16 @@ import { createRequire } from "node:module"; const require = createRequire(import.meta.url); const dest = require.resolve("./leak-fixture-small-ast.js"); // ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the threshold to avoid false positives. -const isASAN = process.execPath.includes("bun-asan"); +// run far higher under ASAN; widen the threshold to avoid false positives. +// Ask the runtime (same as harness.isASAN): debug builds also enable ASAN but +// are named bun-debug, so the executable name alone is not enough. +const isASAN = (() => { + try { + const { isASANEnabled } = require("bun:internal-for-testing"); + if (typeof isASANEnabled === "function") return isASANEnabled(); + } catch {} + return process.execPath.includes("bun-asan"); +})(); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint @@ -17,7 +25,10 @@ for (let i = 0; i < 5; i++) { if (typeof Bun !== "undefined") Bun.gc(true); const baseline = rss(); -for (let i = 0; i < 100000; i++) { +// ASAN builds load modules many times slower, so the driving test scales the +// loop down there via LEAK_ITERATIONS. Everything else keeps the full 100k. +const iterations = Number(process.env.LEAK_ITERATIONS) || 100000; +for (let i = 0; i < iterations; i++) { delete require.cache[dest]; await import(dest); } @@ -26,7 +37,7 @@ if (typeof Bun !== "undefined") Bun.gc(true); setTimeout(() => { let diff = rss() - baseline; diff = (diff / 1024 / 1024) | 0; - console.log({ leaked: diff + " MB" }); + console.log({ leaked: diff + " MB", iterations }); // This test seems to be more flaky on slow filesystems. // This used to be 40 MB, but the original version of Bun which this triggered on would reach 120 MB // so we can increase it to 100 and still catch the leak. @@ -39,7 +50,18 @@ setTimeout(() => { // { // leaked: "38 MB", // } - if (diff >= (isASAN ? 500 : 100)) { + // + // The leak this guards against costs ~1 KB per iteration (100 MB / 100k + // above). Non-ASAN builds run the full loop against the 100 MB bound and + // are what detect it. Under ASAN the free quarantine (256 MB by default) + // and redzones put an iteration-independent floor of several hundred MB + // under RSS, which swamps that signal at any iteration count the lane's + // clock allows, so the ASAN run is a reduced pass that exercises the path + // under the sanitizer and only fails on a gross blowup. At the full 100k + // these are the same 100 MB / 500 MB bounds as before. + const leakAllowance = Math.ceil((60 * iterations) / 100000); + const limit = (isASAN ? 440 : 40) + leakAllowance; + if (diff >= limit) { console.log("\n--fail--\n"); process.exit(1); } else { diff --git a/test/cli/run/require-cache-bug-leak-fixture.js b/test/cli/run/require-cache-bug-leak-fixture.js index e43fa3dbec92..64c87a34ccf2 100644 --- a/test/cli/run/require-cache-bug-leak-fixture.js +++ b/test/cli/run/require-cache-bug-leak-fixture.js @@ -1,7 +1,15 @@ const dest = require.resolve("./require-cache-bug-leak-fixture-large-ast.js"); // ASAN's quarantine retains freed allocations (default 256 MB) so RSS deltas -// run far higher under bun-asan; widen the threshold to avoid false positives. -const isASAN = process.execPath.includes("bun-asan"); +// run far higher under ASAN; widen the threshold to avoid false positives. +// Ask the runtime (same as harness.isASAN): debug builds also enable ASAN but +// are named bun-debug, so the executable name alone is not enough. +const isASAN = (() => { + try { + const { isASANEnabled } = require("bun:internal-for-testing"); + if (typeof isASANEnabled === "function") return isASANEnabled(); + } catch {} + return process.execPath.includes("bun-asan"); +})(); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint diff --git a/test/cli/run/require-cache.test.ts b/test/cli/run/require-cache.test.ts index f95704b24add..804b0de9b22a 100644 --- a/test/cli/run/require-cache.test.ts +++ b/test/cli/run/require-cache.test.ts @@ -6,6 +6,7 @@ import { isASAN, isBroken, isCI, + isDebug, isIntelMacOS, isMacOS, isMusl, @@ -14,6 +15,13 @@ import { } from "harness"; import { join } from "path"; +// The leak tests below measure RSS, which is what detects a leak; their clock +// budgets only need to cover how long the module loads take. Under ASAN or +// debug instrumentation each load is many times slower than in release, so +// those builds share one wide budget in place of the per-block release ones. +const instrumented = isDebug || isASAN; +const leakTimeout = (release: number) => (instrumented ? 120_000 : release); + describe.concurrent("require.cache", () => { test("require.cache is not an empty object literal when inspected", () => { const inspected = Bun.inspect(require.cache); @@ -51,17 +59,19 @@ describe.concurrent("require.cache", () => { }); describe.skipIf(isBroken && isIntelMacOS)("files transpiled and loaded don't leak the output source code", () => { - test("via require() with a lot of long export names", async () => { - let text = ""; - for (let i = 0; i < 10000; i++) { - text += `exports.superDuperExtraCrazyLongNameWowSuchNameLongYouveNeverSeenANameThisLongForACommonJSModuleExport${i} = 1;\n`; - } + test( + "via require() with a lot of long export names", + async () => { + let text = ""; + for (let i = 0; i < 10000; i++) { + text += `exports.superDuperExtraCrazyLongNameWowSuchNameLongYouveNeverSeenANameThisLongForACommonJSModuleExport${i} = 1;\n`; + } - console.log("Text length:", text.length); + console.log("Text length:", text.length); - await using dir = tempDir("require-cache-bug-leak-1", { - "index.js": text, - "require-cache-bug-leak-fixture.js": ` + await using dir = tempDir("require-cache-bug-leak-1", { + "index.js": text, + "require-cache-bug-leak-fixture.js": ` const path = require.resolve("./index.js"); const gc = global.gc || globalThis?.Bun?.gc || (() => {}); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; @@ -97,30 +107,34 @@ describe.concurrent("require.cache", () => { exports.abc = 123; `, - }); - console.log({ dir }); - await using proc = Bun.spawn({ - cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], - env: bunEnv, - stdio: ["inherit", "inherit", "inherit"], - }); - - const exitCode = await proc.exited; - expect(exitCode).toBe(0); - }, 60000); - - test("via await import() with a lot of function calls", async () => { - let text = "function i() { return 1; }\n"; - for (let i = 0; i < 20000; i++) { - text += `i();\n`; - } - text += "exports.forceCommonJS = true;\n"; - - console.log("Text length:", text.length); - - await using dir = tempDir("require-cache-bug-leak-3", { - "index.js": text, - "require-cache-bug-leak-fixture.js": ` + }); + console.log({ dir }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], + env: bunEnv, + stdio: ["inherit", "inherit", "inherit"], + }); + + const exitCode = await proc.exited; + expect(exitCode).toBe(0); + }, + leakTimeout(60_000), + ); + + test( + "via await import() with a lot of function calls", + async () => { + let text = "function i() { return 1; }\n"; + for (let i = 0; i < 20000; i++) { + text += `i();\n`; + } + text += "exports.forceCommonJS = true;\n"; + + console.log("Text length:", text.length); + + await using dir = tempDir("require-cache-bug-leak-3", { + "index.js": text, + "require-cache-bug-leak-fixture.js": ` const path = require.resolve("./index.js"); const gc = global.gc || globalThis?.Bun?.gc || (() => {}); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; @@ -151,26 +165,30 @@ describe.concurrent("require.cache", () => { export default 123; `, - }); - await using proc = Bun.spawn({ - cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], - env: bunEnv, - stdio: ["inherit", "inherit", "inherit"], - }); - - const exitCode = await proc.exited; - expect(exitCode).toBe(0); - }, 60000); // takes 4s on an M1 in release build - - test("via import() with a lot of long export names", async () => { - let text = ""; - for (let i = 0; i < 10000; i++) { - text += `export const superDuperExtraCrazyLongNameWowSuchNameLongYouveNeverSeenANameThisLongForACommonJSModuleExport${i} = 1;\n`; - } - - await using dir = tempDir("require-cache-bug-leak-4", { - "index.js": text, - "require-cache-bug-leak-fixture.js": ` + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], + env: bunEnv, + stdio: ["inherit", "inherit", "inherit"], + }); + + const exitCode = await proc.exited; + expect(exitCode).toBe(0); + }, + leakTimeout(60_000), + ); // takes 4s on an M1 in release build + + test( + "via import() with a lot of long export names", + async () => { + let text = ""; + for (let i = 0; i < 10000; i++) { + text += `export const superDuperExtraCrazyLongNameWowSuchNameLongYouveNeverSeenANameThisLongForACommonJSModuleExport${i} = 1;\n`; + } + + await using dir = tempDir("require-cache-bug-leak-4", { + "index.js": text, + "require-cache-bug-leak-fixture.js": ` const path = require.resolve("./index.js"); const gc = global.gc || globalThis?.Bun?.gc || (() => {}); const rss = process.platform === "darwin" && typeof Bun !== "undefined" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; @@ -201,17 +219,19 @@ describe.concurrent("require.cache", () => { export default 124; `, - }); - console.log({ dir }); - await using proc = Bun.spawn({ - cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], - env: bunEnv, - stdio: ["inherit", "inherit", "inherit"], - }); - - const exitCode = await proc.exited; - expect(exitCode).toBe(0); - }, 60000); + }); + console.log({ dir }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "--smol", join(dir, "require-cache-bug-leak-fixture.js")], + env: bunEnv, + stdio: ["inherit", "inherit", "inherit"], + }); + + const exitCode = await proc.exited; + expect(exitCode).toBe(0); + }, + leakTimeout(60_000), + ); test.todoIf( // Flaky specifically on macOS CI, and on musl-aarch64 under ThinLTO + @@ -277,59 +297,74 @@ describe.concurrent("require.cache", () => { const exitCode = await proc.exited; expect(exitCode).toBe(0); }, - 60000, + leakTimeout(60_000), ); // takes 4s on an M1 in release build }); describe("files transpiled and loaded don't leak the AST", () => { - test("via require()", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "run", join(import.meta.dir, "require-cache-bug-leak-fixture.js")], - env: bunEnv, - stderr: "inherit", - }); - - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); - - expect(stdout.trim()).toEndWith("--pass--"); - expect(exitCode).toBe(0); - }, 20000); - - test("via import()", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "run", join(import.meta.dir, "esm-bug-leak-fixture.mjs")], - env: bunEnv, - stderr: "inherit", - }); - - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); - - expect(stdout.trim()).toEndWith("--pass--"); - expect(exitCode).toBe(0); - }, 20000); + test( + "via require()", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", join(import.meta.dir, "require-cache-bug-leak-fixture.js")], + env: bunEnv, + stderr: "inherit", + }); + + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + + expect(stdout.trim()).toEndWith("--pass--"); + expect(exitCode).toBe(0); + }, + leakTimeout(20_000), + ); + + test( + "via import()", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", join(import.meta.dir, "esm-bug-leak-fixture.mjs")], + env: bunEnv, + stderr: "inherit", + }); + + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + + expect(stdout.trim()).toEndWith("--pass--"); + expect(exitCode).toBe(0); + }, + leakTimeout(20_000), + ); }); - // These tests are extra slow in debug builds + // The import() fixture here loads its module 100k times; under ASAN, where + // the quarantine swamps its RSS signal anyway, it runs a reduced loop + // instead (LEAK_ITERATIONS). Non-ASAN builds keep the full loop and the + // leak detection. describe("files transpiled and loaded don't leak file paths", () => { - test("via require()", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "--smol", "run", join(import.meta.dir, "cjs-fixture-leak-small.js")], - env: bunEnv, - stderr: "inherit", - }); + test( + "via require()", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "--smol", "run", join(import.meta.dir, "cjs-fixture-leak-small.js")], + env: bunEnv, + stderr: "inherit", + }); - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); - expect(stdout.trim()).toEndWith("--pass--"); - expect(exitCode).toBe(0); - }, 30000); + expect(stdout.trim()).toEndWith("--pass--"); + expect(exitCode).toBe(0); + }, + leakTimeout(30_000), + ); test( "via import()", async () => { await using proc = Bun.spawn({ cmd: [bunExe(), "--smol", "run", join(import.meta.dir, "esm-fixture-leak-small.mjs")], - env: bunEnv, + env: { ...bunEnv, LEAK_ITERATIONS: isASAN ? "10000" : "100000" }, stderr: "inherit", }); @@ -339,7 +374,7 @@ describe.concurrent("require.cache", () => { expect(exitCode).toBe(0); }, // TODO: Investigate why this is so slow on Windows - isWindows || isASAN ? 60000 : 30000, + leakTimeout(isWindows ? 60_000 : 30_000), ); }); }); diff --git a/test/js/bun/util/inspect-error-leak.test.js b/test/js/bun/util/inspect-error-leak.test.js index 48619ce18f62..9259ff05136c 100644 --- a/test/js/bun/util/inspect-error-leak.test.js +++ b/test/js/bun/util/inspect-error-leak.test.js @@ -1,26 +1,33 @@ import { expect, test } from "bun:test"; -import { isASAN, rss } from "../../../harness"; +import { isASAN, isDebug, rss } from "../../../harness"; const perBatch = 2000; const repeat = 50; -test("Printing errors does not leak", () => { - function batch() { - for (let i = 0; i < perBatch; i++) { - Bun.inspect(new Error("leak")); +test( + "Printing errors does not leak", + () => { + function batch() { + for (let i = 0; i < perBatch; i++) { + Bun.inspect(new Error("leak")); + } + Bun.gc(true); } - Bun.gc(true); - } - batch(); - const baseline = Math.floor(rss() / 1024); - for (let i = 0; i < repeat; i++) { batch(); - } + const baseline = Math.floor(rss() / 1024); + for (let i = 0; i < repeat; i++) { + batch(); + } - const after = Math.floor(rss() / 1024); - const diff = ((after - baseline) / 1024) | 0; - console.log(`RSS increased by ${diff} MB`); - // ASAN's free quarantine (default 256 MB) plus redzones and glibc page - // retention inflate RSS even when nothing is leaking. - expect(diff, `RSS grew by ${diff} MB after ${perBatch * repeat} iterations`).toBeLessThan(isASAN ? 400 : 10); -}, 10_000); + const after = Math.floor(rss() / 1024); + const diff = ((after - baseline) / 1024) | 0; + console.log(`RSS increased by ${diff} MB`); + // ASAN's free quarantine (default 256 MB) plus redzones and glibc page + // retention inflate RSS even when nothing is leaking. + expect(diff, `RSS grew by ${diff} MB after ${perBatch * repeat} iterations`).toBeLessThan(isASAN ? 400 : 10); + }, + // 100k inspect() calls plus 51 full GCs take ~13s on the release ASAN lane + // and over a minute on a debug ASAN build; the RSS bound above is what + // catches a leak, not the clock. + isDebug || isASAN ? 120_000 : 10_000, +);