diff --git a/src/bun.js/api/ffi.zig b/src/bun.js/api/ffi.zig index 9e061e49a629..f8a9aa6d655f 100644 --- a/src/bun.js/api/ffi.zig +++ b/src/bun.js/api/ffi.zig @@ -1223,8 +1223,7 @@ pub const FFI = struct { if (function.symbol_from_dynamic_library == null) { const ret = global.toInvalidArguments("Symbol \"{s}\" is missing a \"ptr\" field. When using linkSymbols() or CFunction(), you must provide a \"ptr\" field with the memory address of the native function.", .{bun.asByteSlice(function_name)}); for (symbols.values()) |*value| { - allocator.free(@constCast(bun.asByteSlice(value.base_name.?))); - value.arg_types.clearAndFree(allocator); + value.deinit(global); } symbols.clearAndFree(allocator); return ret; @@ -1243,20 +1242,18 @@ pub const FFI = struct { }; switch (function.step) { .failed => |err| { + // Build the error before deinit: `err.msg` is owned by `function.step` + // and `function` is one of `symbols.values()`. + const res = ZigString.init(err.msg).toErrorInstance(global); for (symbols.values()) |*value| { - allocator.free(@constCast(bun.asByteSlice(value.base_name.?))); - value.arg_types.clearAndFree(allocator); + value.deinit(global); } - - const res = ZigString.init(err.msg).toErrorInstance(global); - function.deinit(global); symbols.clearAndFree(allocator); return res; }, .pending => { for (symbols.values()) |*value| { - allocator.free(@constCast(bun.asByteSlice(value.base_name.?))); - value.arg_types.clearAndFree(allocator); + value.deinit(global); } symbols.clearAndFree(allocator); return ZigString.static("Failed to compile (nothing happend!)").toErrorInstance(global); diff --git a/test/js/bun/ffi/ffi-error-messages.test.ts b/test/js/bun/ffi/ffi-error-messages.test.ts index 70dae5e2a7c9..b3efb78af05e 100644 --- a/test/js/bun/ffi/ffi-error-messages.test.ts +++ b/test/js/bun/ffi/ffi-error-messages.test.ts @@ -1,6 +1,6 @@ import { dlopen, linkSymbols } from "bun:ffi"; import { describe, expect, test } from "bun:test"; -import { isArm64, isMusl, isWindows } from "harness"; +import { bunEnv, bunExe, isArm64, isMusl, isWindows } from "harness"; // TinyCC (and all of bun:ffi) is disabled on Windows ARM64 const isFFIUnavailable = isWindows && isArm64; @@ -86,4 +86,42 @@ describe.skipIf(isFFIUnavailable)("FFI error messages", () => { }); }).toThrow('you must provide a "ptr" field with the memory address of the native function.'); }); + + // The symbol name is embedded verbatim into the C source handed to TinyCC. An invalid + // C identifier makes compilation fail and takes the `.failed` cleanup path in + // `linkSymbols`. That path used to free `base_name` for every symbol and then call + // `function.deinit()` on the failing one, freeing its `base_name` a second time. + // Run in a subprocess so the heap-corruption abort (debug/ASAN builds) is observable + // as a non-zero exit instead of tearing down the test runner. + test("linkSymbols cleans up without double-free when TinyCC compilation fails", async () => { + const src = /* js */ ` + const { linkSymbols, JSCallback } = require("bun:ffi"); + const cb = new JSCallback(() => {}, { returns: "void", args: [] }); + let threw = false; + try { + linkSymbols({ + "not a valid C identifier!": { + ptr: cb.ptr, + args: [], + returns: "void", + }, + }); + } catch (e) { + threw = true; + } + cb.close(); + if (!threw) throw new Error("expected linkSymbols to throw"); + console.log("ok"); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("ok\n"); + expect(exitCode).toBe(0); + }); });