Free FFI struct and native resources in finalize()

[robobun]

What

FFI.finalize() at src/bun.js/api/ffi.zig:69 was an empty function:

pub fn finalize(_: *FFI) callconv(.c) void {}

ffi.classes.ts sets finalize: true, so the generated C++ destructor calls into this Zig function expecting it to free the native struct and any owned resources. Since it did nothing:

• the heap FFI struct (from bun.new(FFI, ...)) was leaked on every GC of a dlopen() / cc() / linkSymbols() result
• if the user never called .close() first, the dylib handle, the shared TCC state, and every per-symbol TCC compiler state + arg_types array leaked as well

Repro

const { linkSymbols, JSCallback } = require("bun:ffi");
const cb = new JSCallback(() => 42, { returns: "int32_t", args: [] });
for (let i = 0; i < 20000; i++) {
  linkSymbols({ fn: { returns: "int32_t", args: [], ptr: cb.ptr } });
  if (i % 100 === 0) Bun.gc(true);
}

RSS grows ~315 MB on release / ~230 MB on debug+ASAN over 20k calls.

Fix

Release resources in finalize()

• Extract the cleanup from close() into a private deinit() (same dylib/TCC/functions teardown, guarded by closed).
• finalize() now calls deinit() then bun.destroy(this).
• Drop the unused globalThis parameter from Function.deinit() (it was literally _ = globalThis;) so it can be invoked from the finalizer.
• Switch the cc() allocation site from default_allocator.create(FFI) to bun.new(FFI, ...) so it pairs with bun.destroy().

Keep the wrapper alive while any symbol is reachable

Tearing down TCC/dylib memory is only safe once every JSFFIFunction that points into it is unreachable, but the documented pattern const { symbols: { fn } } = dlopen(...) drops the wrapper immediately. JSFFIFunction held only a raw entry-point pointer with no GC edge back to the wrapper, so GC could run finalize() and unmap the trampoline while fn was still callable (segfaulted test/napi/napi-value-ffi.test.ts).

• Add WriteBarrier<JSObject> m_owner to JSFFIFunction, visited in visitChildren.
• After lib.toJS() in each of dlopen/cc/linkSymbols, point every compiled symbol's m_owner at the FFI wrapper via a new Bun__FFIFunction_setOwner.
• The wrapper is now kept alive as long as any extracted symbol is; finalize() only runs once the last one is collected.

Verification

test/js/bun/ffi/ffi-leak.test.ts has two tests:

1. churns 2000 linkSymbols() calls with periodic GC and asserts RSS growth < 25 MB (before: >30 MB release / >100 MB debug+ASAN; after: <10 MB)
2. extracts a symbol, drops the wrapper, forces GC, then calls the symbol 100× — this segfaulted without the owner edge

BUN_DEBUG_alloc=1 shows a matching destroy(FFI) for every new(FFI), and the wrapper survives GC while an extracted symbol is held:

[alloc] new(FFI) = ...@70e85d0b0050
result: 42
[alloc] destroy(FFI) = ...@70e85d0b0050   // only after fn is dropped

All existing test/js/bun/ffi/* tests pass.

[robobun]

^{Updated 11:36 AM PT - May 4th, 2026}

❌ @autofix-ci[bot], your commit 87e7906 has 3 failures in Build #51174 (All Failures):

• test/cli/install/bun-install-registry.test.ts - code 1 on 🪟 11 aarch64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64-baseline
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 11 aarch64
• test/cli/hot/hot.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 29858

That installs a local version of the PR into your bun-29858 executable, so you can run:

bun-29858 --bun

[github-actions]

Found 1 issue this PR may fix:

1. bun:ffi leaks memory #20845 - Reports bun:ffi leaking memory on repeated FFI usage; commenters confirmed unbounded RSS growth when repeatedly opening/closing libraries — exactly the leak fixed by implementing finalize() to free the FFI struct, dylib handle, and TCC state on GC

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #20845

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@dylan-conway has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 38 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c987c28-ad3b-4084-8fda-3c8140667513

📥 Commits

Reviewing files that changed from the base of the PR and between 088e472 and 62de6ec.

📒 Files selected for processing (4)

• src/bun.js/api/ffi.zig
• src/bun.js/bindings/JSFFIFunction.cpp
• src/bun.js/bindings/JSFFIFunction.h
• test/js/bun/ffi/ffi-leak.test.ts

Walkthrough

The changes centralize FFI resource lifetime management by consolidating deinit logic, updating FFI construction to use bun.new, simplifying Function.deinit signature by removing the globalThis parameter, and adding a new leak regression test.

Changes

Cohort / File(s)	Summary
FFI Lifetime Management src/bun.js/api/ffi.zig	Refactored resource cleanup: finalize now delegates to deinit which handles teardown of dylib, shared_state, and functions map. FFI construction switched to bun.new(). Updated all Function.deinit call sites to remove globalThis argument; simplified Function.deinit signature and unconditional js_function clearing.
FFI Leak Testing test/js/bun/ffi/ffi-leak.test.ts	Added new integration-style leak regression test that spawns a child Bun process to create JSCallback, repeatedly call linkSymbols(), and measure RSS growth under 30MB threshold with 60s timeout.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: freeing FFI struct and native resources in the finalize() function, which is the core fix for the memory leak described in the PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description comprehensively covers what was fixed, why, and how it was verified with specific examples and test results.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Review rate limit: 0/5 reviews remaining, refill in 4 minutes and 38 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bun.js/api/ffi.zig`:
- Around line 74-94: FFI.deinit is double-deinitializing the shared TCC.State
because each Function still holds the same state pointer and Function.deinit()
always calls state.deinit(); fix this by ensuring only the FFI owner
deinitializes the shared_state: before calling each Function.deinit() in
FFI.deinit (and in the error-cleanup paths used by open()/linkSymbols()), null
out or clear the Function.state pointers (or flip an ownership flag on each
Function) so Function.deinit() does not call state.deinit() for the shared
TCC.State; alternatively, add an owned boolean on TCC.State/Function and only
call state.deinit() when owned is true (ensure cc() marks the moved-in state as
owned by FFI and not by individual Functions).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 470be458-a43b-46b9-ab58-a930f438b73d

📥 Commits

Reviewing files that changed from the base of the PR and between 97e4b3f and 088e472.

📒 Files selected for processing (2)

• src/bun.js/api/ffi.zig
• test/js/bun/ffi/ffi-leak.test.ts

[robobun]

Closing as obsolete. This was written against src/bun.js/api/ffi.zig before the Rust port (#30412); the Zig file is now a non-compiled porting reference, so these changes can no longer take effect. The finalize() semantics were since settled in the Rust implementation as an intentional leak when the library was never closed (see the comment in src/runtime/ffi/ffi_body.rs, added in #30875), and the close()-related use-after-free this overlapped with is fixed by #31960.