From bb5e9f9c1b23d61cbdae15bb07126d2c54de5f47 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 04:53:33 +0000 Subject: [PATCH 1/5] ffi: don't free caller-owned memory in toBuffer without a finalizer `bun:ffi.toBuffer(ptr, offset, len)` without an explicit finalizer fell into `JSValue::create_buffer`, which hard-codes `MarkedArrayBuffer_deallocator`. For a borrowed pointer (from `ptr(buffer)` or a dlopen'd symbol) that `mi_free`s storage this Buffer does not own when it is collected: an ASAN bad-free, and a SIGSEGV on release builds (reliably on Windows/macOS where mimalloc override is off). Install a no-op deallocator on the no-finalizer path so the Buffer borrows the pointer instead of freeing it on GC, matching `toArrayBuffer`'s existing behavior. The zero-copy view is preserved; an explicit finalizer still controls disposal and runs exactly once. Tests cover the bad-free (offset 0, interior offset, typed-array source) via subprocess, red on the unpatched build and asserting the caller's memory stays valid, plus a regression that an explicit finalizer is still called exactly once on GC. The `primitives` fixture test drops its no-op deallocator workaround and now exercises the real default path on static native storage. Fixes #35405 Closes #31753 Co-authored-by: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> --- src/runtime/ffi/FFIObject.rs | 55 +++++++----- test/js/bun/ffi/ffi-test.c | 4 - test/js/bun/ffi/ffi.test.js | 157 +++++++++++++++++++++++++++++++++-- 3 files changed, 181 insertions(+), 35 deletions(-) diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 616ef7e3e447..398b175212b3 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -23,9 +23,19 @@ unsafe fn deallocator_from_addr(addr: usize) -> jsc::JSTypedArrayBytesDeallocato unsafe { core::mem::transmute::(addr) } } +/// Bytes deallocator that frees nothing, for a borrowed FFI pointer. +/// `toBuffer(ptr, offset, len)` without an explicit finalizer views caller-owned +/// memory it must NOT free, but the underlying +/// `JSBuffer__bufferFromPointerAndLengthAndDeinit` requires a non-null deallocator +/// for non-empty storage. A deallocator that does nothing satisfies that while +/// leaving the storage caller-owned: GC releases only JSC's view, so no bad-free. +/// Disposal is delegated only when the caller supplies a finalizer. +unsafe extern "C" fn noop_bytes_deallocator(_ptr: *mut c_void, _ctx: *mut c_void) {} + /// Unlike `JSValue::create_buffer` (which hard-codes `MarkedArrayBuffer_deallocator`), -/// this variant passes the caller's (possibly null) deallocator through, so FFI-owned -/// memory is only freed by the user-supplied callback. +/// this variant passes the selected deallocator through: `to_buffer` supplies either +/// the caller's finalizer or `noop_bytes_deallocator` for borrowed storage, so the +/// bytes are only freed when the caller asked for that. #[allow(non_snake_case)] #[inline] fn create_buffer_with_ctx( @@ -43,8 +53,9 @@ fn create_buffer_with_ctx( deallocator: jsc::JSTypedArrayBytesDeallocator, ) -> JSValue; } - // SAFETY: `global` is live; slice describes FFI-owned memory whose - // ownership transfers to JSC (freed via `callback`, or never if None). + // SAFETY: `global` is live; `slice` describes caller-provided memory that stays + // valid for the Buffer's lifetime. `callback` controls disposal, and may be a + // no-op when the storage remains caller-owned (JSC then owns only the view). unsafe { JSBuffer__bufferFromPointerAndLengthAndDeinit( global, @@ -505,12 +516,9 @@ fn ptr_(global_this: &JSGlobalObject, value: JSValue, byte_offset: Option { } }); +// `toBuffer(ptr, offset, len)` without an explicit finalizer used to adopt the +// caller's pointer as owned and install Bun's allocator deallocator, so GC would +// `mi_free` caller-owned memory — an ASAN bad-free / release SIGSEGV. +// These run in a subprocess because the bug crashes the process on unpatched Bun. +describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { + async function runsClean(script) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + const gcLoop = `for (let i = 0; i < 20; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); }`; + + // Forcing GC repeatedly in a subprocess costs a few seconds on a debug build, so + // the default 5s budget is too tight to be reliable. (On an unpatched Bun the child + // SIGSEGVs and then wedges in the crash handler, so there the red signal is this + // timeout rather than the exit-code assertion.) + const GC_TIMEOUT = 20_000; + + // Post-condition on the ACTUAL caller memory the bad-free targets: drop only the + // adopted Buffer, then confirm `original[index]` (the storage `ptr(...)` pointed + // at) is still readable and writable — proving its backing was NOT freed. Then + // drop `original` too, exercising the owner's normal disposal after the borrowed + // view was collected — on an unpatched build the earlier invalid free has already + // corrupted that ownership path. That turns "the process didn't abort" into "the + // caller memory survived". + const originalSurvives = (index, expected) => ` + adopted = null; + ${gcLoop} + if (original[${index}] !== ${expected}) throw new Error("caller memory corrupted after adopted GC: " + original[${index}]); + original[${index}] = 0x55; + if (original[${index}] !== 0x55) throw new Error("caller memory not writable after adopted GC"); + original = null; + ${gcLoop} + `; + + it( + "toBuffer(ptr(buffer)) does not free caller-owned memory on GC", + async () => { + const { stdout, exitCode } = await runsClean(` + import { ptr, toBuffer } from "bun:ffi"; + let original = Buffer.alloc(64, 0x41); + let adopted = toBuffer(ptr(original), 0, 64); + if (adopted[0] !== 0x41) throw new Error("expected a zero-copy view"); + adopted[0] = 0x42; + if (original[0] !== 0x42) throw new Error("expected an aliasing view"); + ${originalSurvives(0, "0x42")} + console.log("survived-gc"); + `); + expect(stdout).toContain("survived-gc"); + expect(exitCode).toBe(0); + }, + GC_TIMEOUT, + ); + + it( + "toBuffer(ptr(buffer), offset) does not free an interior pointer on GC", + async () => { + // The adopted view starts at original[8], so both the aliasing check and the + // post-GC survival check must inspect that byte, not original[0]. + const { stdout, exitCode } = await runsClean(` + import { ptr, toBuffer } from "bun:ffi"; + let original = Buffer.alloc(64, 0x41); + let adopted = toBuffer(ptr(original), 8, 48); + if (adopted[0] !== 0x41) throw new Error("expected a zero-copy view"); + adopted[0] = 0x42; + if (original[8] !== 0x42) throw new Error("expected a view aliasing original[8]"); + ${originalSurvives(8, "0x42")} + console.log("survived-gc"); + `); + expect(stdout).toContain("survived-gc"); + expect(exitCode).toBe(0); + }, + GC_TIMEOUT, + ); + + it( + "toBuffer(ptr(typedArray)) does not free caller-owned memory on GC", + async () => { + const { stdout, exitCode } = await runsClean(` + import { ptr, toBuffer } from "bun:ffi"; + let original = new Uint8Array(64).fill(0x41); + let adopted = toBuffer(ptr(original), 0, 64); + ${originalSurvives(0, "0x41")} + console.log("survived-gc"); + `); + expect(stdout).toContain("survived-gc"); + expect(exitCode).toBe(0); + }, + GC_TIMEOUT, + ); + + // Regression (the #1 risk): the bad-free fix must NOT change the explicit-finalizer + // path — when the caller supplies a finalizer, it still controls disposal and the + // deallocator is invoked exactly once on GC. Self-contained via cc() (TinyCC) so the + // deallocator's call count is isolated from the shared ffi-test fixture's counter. + it( + "toBuffer with an explicit finalizer calls the deallocator exactly once on GC", + () => { + using dir = tempDir("ffi-tobuffer-finalizer", { + "dealloc.c": ` + static int ffi_called = 0; + static void* ffi_last_ptr = 0; + static unsigned char ffi_buf[128]; + void ffi_dealloc(void* p, void* ctx) { (void)ctx; ffi_last_ptr = p; ffi_called++; } + void* ffi_get_dealloc(void) { return (void*)&ffi_dealloc; } + void* ffi_get_buf(void) { return (void*)ffi_buf; } + void* ffi_get_last_ptr(void) { return ffi_last_ptr; } + int ffi_get_called(void) { return ffi_called; } + `, + }); + const { symbols } = cc({ + source: `${String(dir)}/dealloc.c`, + symbols: { + ffi_get_dealloc: { args: [], returns: "ptr" }, + ffi_get_buf: { args: [], returns: "ptr" }, + ffi_get_last_ptr: { args: [], returns: "ptr" }, + ffi_get_called: { args: [], returns: "int" }, + }, + }); + const bufPtr = symbols.ffi_get_buf(); + let buf = toBuffer(bufPtr, 0, 128, symbols.ffi_get_dealloc()); + expect(buf.length).toBe(128); + expect(symbols.ffi_get_called()).toBe(0); // not called during construction + buf = null; + // Await the collection rather than assuming one pass suffices: a single + // Bun.gc(true) can still see `buf` conservatively from the stack. + for (let i = 0; i < 20 && symbols.ffi_get_called() === 0; i++) { + Bun.gc(true); + Buffer.alloc(1024 * 1024); + } + expect(symbols.ffi_get_called()).toBe(1); // called exactly once on GC + expect(symbols.ffi_get_last_ptr()).toBe(bufPtr); // with the buffer's own pointer + Bun.gc(true); + expect(symbols.ffi_get_called()).toBe(1); // not called again + }, + GC_TIMEOUT, + ); +}); + describe.skipIf(!FFI_FIXTURE_PATH)("engine-native FFI (single implementation)", () => { const lib = FFI_FIXTURE_PATH; it("linkSymbols() binds and calls symbols from raw pointers", () => { From a475c24bdcecf11ffc785338262436ae2d2574da Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:14:18 +0000 Subject: [PATCH 2/5] test(ffi): use combined-object assertions and the compiled fixture for the finalizer guard The subprocess tests now assert the full {stdout, stderr, exitCode} object so a failure diff carries the child's crash output instead of a bare empty-string mismatch, matching the rest of the file. The explicit-finalizer regression guard switches from cc() (TinyCC) to the compiled ffi-test fixture's getDeallocatorCallback/getDeallocatorBuffer/ getDeallocatorCalledCount helpers. Every in-process cc() invocation in the repo is gated on isASAN, and the debian x64-asan lane reported a generate_symbols leak from this one. The fixture-based test runs under ASAN and the helpers already reset the counter, so no isolation is lost. Co-authored-by: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> --- test/js/bun/ffi/ffi.test.js | 72 +++++++++++++++---------------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index f8363100a5ce..a559bedb583e 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -1398,7 +1398,8 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { it( "toBuffer(ptr(buffer)) does not free caller-owned memory on GC", async () => { - const { stdout, exitCode } = await runsClean(` + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = Buffer.alloc(64, 0x41); let adopted = toBuffer(ptr(original), 0, 64); @@ -1407,9 +1408,8 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { if (original[0] !== 0x42) throw new Error("expected an aliasing view"); ${originalSurvives(0, "0x42")} console.log("survived-gc"); - `); - expect(stdout).toContain("survived-gc"); - expect(exitCode).toBe(0); + `), + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); }, GC_TIMEOUT, ); @@ -1419,7 +1419,8 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { async () => { // The adopted view starts at original[8], so both the aliasing check and the // post-GC survival check must inspect that byte, not original[0]. - const { stdout, exitCode } = await runsClean(` + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = Buffer.alloc(64, 0x41); let adopted = toBuffer(ptr(original), 8, 48); @@ -1428,9 +1429,8 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { if (original[8] !== 0x42) throw new Error("expected a view aliasing original[8]"); ${originalSurvives(8, "0x42")} console.log("survived-gc"); - `); - expect(stdout).toContain("survived-gc"); - expect(exitCode).toBe(0); + `), + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); }, GC_TIMEOUT, ); @@ -1438,62 +1438,48 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { it( "toBuffer(ptr(typedArray)) does not free caller-owned memory on GC", async () => { - const { stdout, exitCode } = await runsClean(` + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = new Uint8Array(64).fill(0x41); let adopted = toBuffer(ptr(original), 0, 64); ${originalSurvives(0, "0x41")} console.log("survived-gc"); - `); - expect(stdout).toContain("survived-gc"); - expect(exitCode).toBe(0); + `), + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); }, GC_TIMEOUT, ); // Regression (the #1 risk): the bad-free fix must NOT change the explicit-finalizer - // path — when the caller supplies a finalizer, it still controls disposal and the - // deallocator is invoked exactly once on GC. Self-contained via cc() (TinyCC) so the - // deallocator's call count is isolated from the shared ffi-test fixture's counter. - it( + // path. When the caller supplies a finalizer, it still controls disposal and the + // deallocator is invoked exactly once on GC. Uses the compiled fixture's + // deallocator-counter helpers (getDeallocatorBuffer/getDeallocatorCallback both + // reset the counter, so the count is isolated to this test). + it.skipIf(!FFI_FIXTURE_PATH)( "toBuffer with an explicit finalizer calls the deallocator exactly once on GC", () => { - using dir = tempDir("ffi-tobuffer-finalizer", { - "dealloc.c": ` - static int ffi_called = 0; - static void* ffi_last_ptr = 0; - static unsigned char ffi_buf[128]; - void ffi_dealloc(void* p, void* ctx) { (void)ctx; ffi_last_ptr = p; ffi_called++; } - void* ffi_get_dealloc(void) { return (void*)&ffi_dealloc; } - void* ffi_get_buf(void) { return (void*)ffi_buf; } - void* ffi_get_last_ptr(void) { return ffi_last_ptr; } - int ffi_get_called(void) { return ffi_called; } - `, - }); - const { symbols } = cc({ - source: `${String(dir)}/dealloc.c`, - symbols: { - ffi_get_dealloc: { args: [], returns: "ptr" }, - ffi_get_buf: { args: [], returns: "ptr" }, - ffi_get_last_ptr: { args: [], returns: "ptr" }, - ffi_get_called: { args: [], returns: "int" }, - }, + const { + symbols: { getDeallocatorCallback, getDeallocatorBuffer, getDeallocatorCalledCount }, + } = dlopen(FFI_FIXTURE_PATH, { + getDeallocatorCallback: { args: [], returns: "ptr" }, + getDeallocatorBuffer: { args: [], returns: "ptr" }, + getDeallocatorCalledCount: { args: [], returns: "int" }, }); - const bufPtr = symbols.ffi_get_buf(); - let buf = toBuffer(bufPtr, 0, 128, symbols.ffi_get_dealloc()); + const bufPtr = getDeallocatorBuffer(); + let buf = toBuffer(bufPtr, 0, 128, getDeallocatorCallback()); expect(buf.length).toBe(128); - expect(symbols.ffi_get_called()).toBe(0); // not called during construction + expect(getDeallocatorCalledCount()).toBe(0); // not called during construction buf = null; // Await the collection rather than assuming one pass suffices: a single // Bun.gc(true) can still see `buf` conservatively from the stack. - for (let i = 0; i < 20 && symbols.ffi_get_called() === 0; i++) { + for (let i = 0; i < 20 && getDeallocatorCalledCount() === 0; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); } - expect(symbols.ffi_get_called()).toBe(1); // called exactly once on GC - expect(symbols.ffi_get_last_ptr()).toBe(bufPtr); // with the buffer's own pointer + expect(getDeallocatorCalledCount()).toBe(1); // called exactly once on GC Bun.gc(true); - expect(symbols.ffi_get_called()).toBe(1); // not called again + expect(getDeallocatorCalledCount()).toBe(1); // not called again }, GC_TIMEOUT, ); From 0f5fac6ef53272296a580e92caafef3bc2071ecb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:17:47 +0000 Subject: [PATCH 3/5] ffi: trim FFIObject.rs comments to the essentials Co-authored-by: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> --- src/runtime/ffi/FFIObject.rs | 37 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 398b175212b3..0c48941d71e4 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -23,19 +23,12 @@ unsafe fn deallocator_from_addr(addr: usize) -> jsc::JSTypedArrayBytesDeallocato unsafe { core::mem::transmute::(addr) } } -/// Bytes deallocator that frees nothing, for a borrowed FFI pointer. -/// `toBuffer(ptr, offset, len)` without an explicit finalizer views caller-owned -/// memory it must NOT free, but the underlying -/// `JSBuffer__bufferFromPointerAndLengthAndDeinit` requires a non-null deallocator -/// for non-empty storage. A deallocator that does nothing satisfies that while -/// leaving the storage caller-owned: GC releases only JSC's view, so no bad-free. -/// Disposal is delegated only when the caller supplies a finalizer. +/// Frees nothing. `JSBuffer__bufferFromPointerAndLengthAndDeinit` asserts a non-null +/// deallocator for `len > 0`, so a borrowed view supplies this instead of `None`. unsafe extern "C" fn noop_bytes_deallocator(_ptr: *mut c_void, _ctx: *mut c_void) {} /// Unlike `JSValue::create_buffer` (which hard-codes `MarkedArrayBuffer_deallocator`), -/// this variant passes the selected deallocator through: `to_buffer` supplies either -/// the caller's finalizer or `noop_bytes_deallocator` for borrowed storage, so the -/// bytes are only freed when the caller asked for that. +/// this passes the caller's deallocator through so FFI bytes are only freed when asked. #[allow(non_snake_case)] #[inline] fn create_buffer_with_ctx( @@ -53,9 +46,8 @@ fn create_buffer_with_ctx( deallocator: jsc::JSTypedArrayBytesDeallocator, ) -> JSValue; } - // SAFETY: `global` is live; `slice` describes caller-provided memory that stays - // valid for the Buffer's lifetime. `callback` controls disposal, and may be a - // no-op when the storage remains caller-owned (JSC then owns only the view). + // SAFETY: `global` is live; `slice` stays valid for the Buffer's lifetime. + // `callback` controls disposal (a no-op when the storage stays caller-owned). unsafe { JSBuffer__bufferFromPointerAndLengthAndDeinit( global, @@ -513,12 +505,8 @@ fn ptr_(global_this: &JSGlobalObject, value: JSValue, byte_offset: Option Date: Fri, 31 Jul 2026 05:38:07 +0000 Subject: [PATCH 4/5] test(ffi): address self-review - make the three independent subprocess tests `it.concurrent` - the explicit-finalizer GC poll is now async with a yield between collections and a 100-iteration ceiling, matching the repo's gcUntil shape - drop the incorrect 'wedges in the crash handler' parenthetical; the unpatched child exits non-zero promptly (139 release / ASAN abort), so the toEqual mismatch is the red signal Co-authored-by: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> --- test/js/bun/ffi/ffi.test.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index a559bedb583e..f4f7e7bf0f3a 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -1373,9 +1373,7 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { const gcLoop = `for (let i = 0; i < 20; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); }`; // Forcing GC repeatedly in a subprocess costs a few seconds on a debug build, so - // the default 5s budget is too tight to be reliable. (On an unpatched Bun the child - // SIGSEGVs and then wedges in the crash handler, so there the red signal is this - // timeout rather than the exit-code assertion.) + // the default 5s budget is too tight to be reliable. const GC_TIMEOUT = 20_000; // Post-condition on the ACTUAL caller memory the bad-free targets: drop only the @@ -1395,7 +1393,7 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { ${gcLoop} `; - it( + it.concurrent( "toBuffer(ptr(buffer)) does not free caller-owned memory on GC", async () => { expect( @@ -1414,7 +1412,7 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { GC_TIMEOUT, ); - it( + it.concurrent( "toBuffer(ptr(buffer), offset) does not free an interior pointer on GC", async () => { // The adopted view starts at original[8], so both the aliasing check and the @@ -1435,7 +1433,7 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { GC_TIMEOUT, ); - it( + it.concurrent( "toBuffer(ptr(typedArray)) does not free caller-owned memory on GC", async () => { expect( @@ -1458,7 +1456,7 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { // reset the counter, so the count is isolated to this test). it.skipIf(!FFI_FIXTURE_PATH)( "toBuffer with an explicit finalizer calls the deallocator exactly once on GC", - () => { + async () => { const { symbols: { getDeallocatorCallback, getDeallocatorBuffer, getDeallocatorCalledCount }, } = dlopen(FFI_FIXTURE_PATH, { @@ -1466,16 +1464,19 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { getDeallocatorBuffer: { args: [], returns: "ptr" }, getDeallocatorCalledCount: { args: [], returns: "int" }, }); - const bufPtr = getDeallocatorBuffer(); - let buf = toBuffer(bufPtr, 0, 128, getDeallocatorCallback()); - expect(buf.length).toBe(128); - expect(getDeallocatorCalledCount()).toBe(0); // not called during construction - buf = null; - // Await the collection rather than assuming one pass suffices: a single - // Bun.gc(true) can still see `buf` conservatively from the stack. - for (let i = 0; i < 20 && getDeallocatorCalledCount() === 0; i++) { + (() => { + const bufPtr = getDeallocatorBuffer(); + let buf = toBuffer(bufPtr, 0, 128, getDeallocatorCallback()); + expect(buf.length).toBe(128); + expect(getDeallocatorCalledCount()).toBe(0); // not called during construction + buf = null; + })(); + // Yield between collections so the frame that held `buf` is popped before the + // conservative scan; a synchronous loop can keep a single cell pinned. + for (let i = 0; i < 100 && getDeallocatorCalledCount() === 0; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); + await Bun.sleep(0); } expect(getDeallocatorCalledCount()).toBe(1); // called exactly once on GC Bun.gc(true); From 032d4b913aa7628fd1803b8304afc1de5808040f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:45:24 +0000 Subject: [PATCH 5/5] test(ffi): drop per-test timeout, trim test comments, assert aliasing in the typed-array case Measured debug+ASAN runtime is ~1s per subprocess test (concurrent), well under the default budget. Co-authored-by: Steven Zimmerman <15812269+EffortlessSteven@users.noreply.github.com> --- test/js/bun/ffi/ffi.test.js | 84 +++++++++++++------------------------ 1 file changed, 28 insertions(+), 56 deletions(-) diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index f4f7e7bf0f3a..aad15470fb5b 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -1354,10 +1354,8 @@ describe.if(!!libPath)("can open more than 63 symbols via", () => { } }); -// `toBuffer(ptr, offset, len)` without an explicit finalizer used to adopt the -// caller's pointer as owned and install Bun's allocator deallocator, so GC would -// `mi_free` caller-owned memory — an ASAN bad-free / release SIGSEGV. -// These run in a subprocess because the bug crashes the process on unpatched Bun. +// oven-sh/bun#35405: toBuffer without a finalizer used to mi_free caller-owned +// memory on GC. Subprocess because unpatched builds crash. describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { async function runsClean(script) { await using proc = Bun.spawn({ @@ -1372,17 +1370,8 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { const gcLoop = `for (let i = 0; i < 20; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); }`; - // Forcing GC repeatedly in a subprocess costs a few seconds on a debug build, so - // the default 5s budget is too tight to be reliable. - const GC_TIMEOUT = 20_000; - - // Post-condition on the ACTUAL caller memory the bad-free targets: drop only the - // adopted Buffer, then confirm `original[index]` (the storage `ptr(...)` pointed - // at) is still readable and writable — proving its backing was NOT freed. Then - // drop `original` too, exercising the owner's normal disposal after the borrowed - // view was collected — on an unpatched build the earlier invalid free has already - // corrupted that ownership path. That turns "the process didn't abort" into "the - // caller memory survived". + // Drops the borrowed view first, then the owner, so an invalid free shows up as + // corrupted caller memory rather than only as a crash. const originalSurvives = (index, expected) => ` adopted = null; ${gcLoop} @@ -1393,11 +1382,9 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { ${gcLoop} `; - it.concurrent( - "toBuffer(ptr(buffer)) does not free caller-owned memory on GC", - async () => { - expect( - await runsClean(` + it.concurrent("toBuffer(ptr(buffer)) does not free caller-owned memory on GC", async () => { + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = Buffer.alloc(64, 0x41); let adopted = toBuffer(ptr(original), 0, 64); @@ -1407,18 +1394,12 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { ${originalSurvives(0, "0x42")} console.log("survived-gc"); `), - ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); - }, - GC_TIMEOUT, - ); + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); + }); - it.concurrent( - "toBuffer(ptr(buffer), offset) does not free an interior pointer on GC", - async () => { - // The adopted view starts at original[8], so both the aliasing check and the - // post-GC survival check must inspect that byte, not original[0]. - expect( - await runsClean(` + it.concurrent("toBuffer(ptr(buffer), offset) does not free an interior pointer on GC", async () => { + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = Buffer.alloc(64, 0x41); let adopted = toBuffer(ptr(original), 8, 48); @@ -1428,32 +1409,26 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { ${originalSurvives(8, "0x42")} console.log("survived-gc"); `), - ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); - }, - GC_TIMEOUT, - ); + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); + }); - it.concurrent( - "toBuffer(ptr(typedArray)) does not free caller-owned memory on GC", - async () => { - expect( - await runsClean(` + it.concurrent("toBuffer(ptr(typedArray)) does not free caller-owned memory on GC", async () => { + expect( + await runsClean(` import { ptr, toBuffer } from "bun:ffi"; let original = new Uint8Array(64).fill(0x41); let adopted = toBuffer(ptr(original), 0, 64); - ${originalSurvives(0, "0x41")} + if (adopted[0] !== 0x41) throw new Error("expected a zero-copy view"); + adopted[0] = 0x42; + if (original[0] !== 0x42) throw new Error("expected an aliasing view"); + ${originalSurvives(0, "0x42")} console.log("survived-gc"); `), - ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); - }, - GC_TIMEOUT, - ); + ).toEqual({ stdout: "survived-gc\n", stderr: "", exitCode: 0 }); + }); - // Regression (the #1 risk): the bad-free fix must NOT change the explicit-finalizer - // path. When the caller supplies a finalizer, it still controls disposal and the - // deallocator is invoked exactly once on GC. Uses the compiled fixture's - // deallocator-counter helpers (getDeallocatorBuffer/getDeallocatorCallback both - // reset the counter, so the count is isolated to this test). + // Regression guard: an explicit finalizer still controls disposal and runs exactly + // once on GC. getDeallocatorBuffer/getDeallocatorCallback each reset the counter. it.skipIf(!FFI_FIXTURE_PATH)( "toBuffer with an explicit finalizer calls the deallocator exactly once on GC", async () => { @@ -1468,21 +1443,18 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { const bufPtr = getDeallocatorBuffer(); let buf = toBuffer(bufPtr, 0, 128, getDeallocatorCallback()); expect(buf.length).toBe(128); - expect(getDeallocatorCalledCount()).toBe(0); // not called during construction + expect(getDeallocatorCalledCount()).toBe(0); buf = null; })(); - // Yield between collections so the frame that held `buf` is popped before the - // conservative scan; a synchronous loop can keep a single cell pinned. for (let i = 0; i < 100 && getDeallocatorCalledCount() === 0; i++) { Bun.gc(true); Buffer.alloc(1024 * 1024); await Bun.sleep(0); } - expect(getDeallocatorCalledCount()).toBe(1); // called exactly once on GC + expect(getDeallocatorCalledCount()).toBe(1); Bun.gc(true); - expect(getDeallocatorCalledCount()).toBe(1); // not called again + expect(getDeallocatorCalledCount()).toBe(1); }, - GC_TIMEOUT, ); });