fix(napi): tie external buffer finalizer to ArrayBuffer lifecycle

This fixes a race condition in `napi_create_external_buffer` where the
finalizer was added separately via `vm.heap.addFinalizer`, allowing
native code to free/reuse the underlying memory while JavaScript still
held a reference to the ArrayBuffer, causing data corruption.

The fix ties the finalizer directly to the ArrayBuffer via
`createFromBytes`, matching the pattern used in
`napi_create_external_arraybuffer`.

Fixes #26446

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

src/bun.js/bindings/napi.cpp

@@ -2044,20 +2044,14 @@ extern "C" napi_status napi_create_external_buffer(napi_env env, size_t length,
         NAPI_RETURN_SUCCESS(env);
     }
 
-    auto arrayBuffer = ArrayBuffer::createFromBytes({ reinterpret_cast<const uint8_t*>(data), length }, createSharedTask<void(void*)>([](void*) {
-        // do nothing
+    auto arrayBuffer = ArrayBuffer::createFromBytes({ reinterpret_cast<const uint8_t*>(data), length }, createSharedTask<void(void*)>([envRef = WTF::Ref<NapiEnv>(*env), finalize_hint, finalize_cb](void* p) {
+        NAPI_LOG("external buffer finalizer");
+        envRef->doFinalizer(finalize_cb, p, finalize_hint);
     }));
 
     auto* buffer = JSC::JSUint8Array::create(globalObject, subclassStructure, WTF::move(arrayBuffer), 0, length);
     NAPI_RETURN_IF_EXCEPTION(env);
 
-    // setup finalizer after creating the array. if it throws callers of napi_create_external_buffer are expected
-    // to free input
-    vm.heap.addFinalizer(buffer, [env = WTF::Ref<NapiEnv>(*env), finalize_cb, data, finalize_hint](JSCell* cell) -> void {
-        NAPI_LOG("external buffer finalizer");
-        env->doFinalizer(finalize_cb, data, finalize_hint);
-    });
-
     *result = toNapi(buffer, globalObject);
     NAPI_RETURN_SUCCESS(env);
 }

src/install/PackageManager/CommandLineArguments.zig

@@ -119,13 +119,11 @@ pub const unlink_params: []const ParamType = &(shared_params ++ [_]ParamType{
 const patch_params: []const ParamType = &(shared_params ++ [_]ParamType{
     clap.parseParam("<POS> ...                         \"name\" of the package to patch") catch unreachable,
     clap.parseParam("--commit                         Install a package containing modifications in `dir`") catch unreachable,
-    clap.parseParam("--preview                        Print the diff without saving it (implies --commit)") catch unreachable,
     clap.parseParam("--patches-dir <dir>                    The directory to put the patch file in (only if --commit is used)") catch unreachable,
 });
 
 const patch_commit_params: []const ParamType = &(shared_params ++ [_]ParamType{
     clap.parseParam("<POS> ...                         \"dir\" containing changes to a package") catch unreachable,
-    clap.parseParam("--preview                        Print the diff without saving it") catch unreachable,
     clap.parseParam("--patches-dir <dir>                    The directory to put the patch file") catch unreachable,
 });
 
@@ -283,7 +281,6 @@ const PatchOpts = union(enum) {
     patch: struct {},
     commit: struct {
         patches_dir: []const u8 = "patches",
-        preview: bool = false,
     },
 };
 
@@ -383,9 +380,6 @@ pub fn printHelp(subcommand: Subcommand) void {
                 \\  <d>Generate a patch file for changes made to jquery<r>
                 \\  <b><green>bun patch --commit 'node_modules/jquery'<r>
                 \\
-                \\  <d>Preview a patch without saving it<r>
-                \\  <b><green>bun patch --preview 'node_modules/jquery'<r>
-                \\
                 \\  <d>Generate a patch file in a custom directory for changes made to jquery<r>
                 \\  <b><green>bun patch --patches-dir 'my-patches' 'node_modules/jquery'<r>
                 \\
@@ -414,9 +408,6 @@ pub fn printHelp(subcommand: Subcommand) void {
                 \\  <d>Generate a patch in the default "./patches" directory for changes in "./node_modules/jquery"<r>
                 \\  <b><green>bun patch-commit 'node_modules/jquery'<r>
                 \\
-                \\  <d>Preview a patch without saving it<r>
-                \\  <b><green>bun patch-commit --preview 'node_modules/jquery'<r>
-                \\
                 \\  <d>Generate a patch in a custom directory ("./my-patches")<r>
                 \\  <b><green>bun patch-commit --patches-dir 'my-patches' 'node_modules/jquery'<r>
                 \\
@@ -943,12 +934,10 @@ pub fn parse(allocator: std.mem.Allocator, comptime subcommand: Subcommand) !Com
 
     if (subcommand == .patch) {
         const patch_commit = args.flag("--commit");
-        const patch_preview = args.flag("--preview");
-        if (patch_commit or patch_preview) {
+        if (patch_commit) {
             cli.patch = .{
                 .commit = .{
                     .patches_dir = args.option("--patches-dir") orelse "patches",
-                    .preview = patch_preview,
                 },
             };
         } else {
@@ -961,7 +950,6 @@ pub fn parse(allocator: std.mem.Allocator, comptime subcommand: Subcommand) !Com
         cli.patch = .{
             .commit = .{
                 .patches_dir = args.option("--patches-dir") orelse "patches",
-                .preview = args.flag("--preview"),
             },
         };
     }

src/install/PackageManager/PackageManagerOptions.zig

@@ -33,7 +33,6 @@ patch_features: union(enum) {
     patch: struct {},
     commit: struct {
         patches_dir: string,
-        preview: bool = false,
     },
 } = .{ .nothing = .{} },
 
@@ -649,11 +648,10 @@ pub fn load(
             .patch => {
                 this.patch_features = .{ .patch = .{} };
             },
-            .commit => |commit_opts| {
+            .commit => {
                 this.patch_features = .{
                     .commit = .{
-                        .patches_dir = commit_opts.patches_dir,
-                        .preview = commit_opts.preview,
+                        .patches_dir = cli.patch.commit.patches_dir,
                     },
                 };
             },

src/install/PackageManager/patchPackage.zig

@@ -391,16 +391,6 @@ pub fn doPatchCommit(
     };
     defer patchfile_contents.deinit();
 
-    // In preview mode, print the diff to stdout and return without modifying any files
-    if (manager.options.patch_features.commit.preview) {
-        Output.writer().writeAll(patchfile_contents.items) catch |e| {
-            Output.err(e, "failed to write patch to stdout", .{});
-            Global.crash();
-        };
-        Output.flush();
-        return null;
-    }
-
     // write the patch contents to temp file then rename
     var tmpname_buf: [1024]u8 = undefined;
     const tempfile_name = try bun.fs.FileSystem.tmpname("tmp", &tmpname_buf, bun.fastRandom());

test/cli/install/bun-patch.test.ts

@@ -816,146 +816,4 @@ module.exports = function isOdd() {
       expect(stdout.toString()).toBe("true\n");
     }
   });
-
-  describe("--preview flag", async () => {
-    test("should print diff without saving patch file", async () => {
-      const tempdir = tempDirWithFiles("preview", {
-        "package.json": JSON.stringify({
-          name: "bun-patch-preview-test",
-          module: "index.ts",
-          type: "module",
-          dependencies: {
-            "is-even": "1.0.0",
-          },
-        }),
-        "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(420))`,
-      });
-
-      // Install dependencies
-      expectNoError(await $`${bunExe()} i`.env(bunEnv).cwd(tempdir));
-
-      // Prepare the package for patching
-      expectNoError(await $`${bunExe()} patch is-even@1.0.0`.env(bunEnv).cwd(tempdir));
-
-      // Make a change to the package
-      const patchedCode = /* ts */ `/*!
-* is-even <https://github.com/jonschlinkert/is-even>
-*
-* Copyright (c) 2015, 2017, Jon Schlinkert.
-* Released under the MIT License.
-*/
-
-'use strict';
-
-var isOdd = require('is-odd');
-
-module.exports = function isEven(i) {
-  console.log("Preview test!")
-  return !isOdd(i);
-};
-`;
-      await $`echo ${patchedCode} > node_modules/is-even/index.js`.env(bunEnv).cwd(tempdir);
-
-      // Run bun patch --preview
-      const { stdout, stderr } = await $`${bunExe()} patch --preview node_modules/is-even`.env(bunEnv).cwd(tempdir);
-      expect(stderr.toString()).not.toContain("error");
-
-      // The stdout should contain the diff
-      expect(stdout.toString()).toContain("diff --git");
-      expect(stdout.toString()).toContain("Preview test!");
-      expect(stdout.toString()).toContain("index.js");
-
-      // The patches directory should NOT exist (no file saved)
-      const { exitCode } = await $`test -d patches`.env(bunEnv).cwd(tempdir).throws(false);
-      expect(exitCode).not.toBe(0);
-
-      // package.json should NOT have patchedDependencies
-      const packageJson = await $`cat package.json`.cwd(tempdir).env(bunEnv).json();
-      expect(packageJson.patchedDependencies).toBeUndefined();
-    });
-
-    test("should work with bun patch-commit --preview", async () => {
-      const tempdir = tempDirWithFiles("preview2", {
-        "package.json": JSON.stringify({
-          name: "bun-patch-preview-test-2",
-          module: "index.ts",
-          type: "module",
-          dependencies: {
-            "is-even": "1.0.0",
-          },
-        }),
-        "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(420))`,
-      });
-
-      // Install dependencies
-      expectNoError(await $`${bunExe()} i`.env(bunEnv).cwd(tempdir));
-
-      // Prepare the package for patching
-      expectNoError(await $`${bunExe()} patch is-even@1.0.0`.env(bunEnv).cwd(tempdir));
-
-      // Make a change to the package
-      const patchedCode = /* ts */ `/*!
-* is-even <https://github.com/jonschlinkert/is-even>
-*
-* Copyright (c) 2015, 2017, Jon Schlinkert.
-* Released under the MIT License.
-*/
-
-'use strict';
-
-var isOdd = require('is-odd');
-
-module.exports = function isEven(i) {
-  console.log("patch-commit preview!")
-  return !isOdd(i);
-};
-`;
-      await $`echo ${patchedCode} > node_modules/is-even/index.js`.env(bunEnv).cwd(tempdir);
-
-      // Run bun patch-commit --preview
-      const { stdout, stderr } = await $`${bunExe()} patch-commit --preview node_modules/is-even`
-        .env(bunEnv)
-        .cwd(tempdir);
-      expect(stderr.toString()).not.toContain("error");
-
-      // The stdout should contain the diff
-      expect(stdout.toString()).toContain("diff --git");
-      expect(stdout.toString()).toContain("patch-commit preview!");
-
-      // The patches directory should NOT exist (no file saved)
-      const { exitCode } = await $`test -d patches`.env(bunEnv).cwd(tempdir).throws(false);
-      expect(exitCode).not.toBe(0);
-
-      // package.json should NOT have patchedDependencies
-      const packageJson = await $`cat package.json`.cwd(tempdir).env(bunEnv).json();
-      expect(packageJson.patchedDependencies).toBeUndefined();
-    });
-
-    test("--preview should show no changes when package is unmodified", async () => {
-      const tempdir = tempDirWithFiles("preview3", {
-        "package.json": JSON.stringify({
-          name: "bun-patch-preview-test-3",
-          module: "index.ts",
-          type: "module",
-          dependencies: {
-            "is-even": "1.0.0",
-          },
-        }),
-        "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(420))`,
-      });
-
-      // Install dependencies
-      expectNoError(await $`${bunExe()} i`.env(bunEnv).cwd(tempdir));
-
-      // Prepare the package for patching (but don't make any changes)
-      expectNoError(await $`${bunExe()} patch is-even@1.0.0`.env(bunEnv).cwd(tempdir));
-
-      // Run bun patch --preview without making changes
-      const { stdout, stderr } = await $`${bunExe()} patch --preview node_modules/is-even`.env(bunEnv).cwd(tempdir);
-      expect(stderr.toString()).not.toContain("error");
-
-      // Should indicate no changes
-      expect(stdout.toString()).toContain("No changes detected");
-    });
-  });
 });

test/napi/napi-app/module.js

@@ -768,6 +768,44 @@ nativeTests.test_ref_unref_underflow = () => {
   }
 };
 
+// Test for napi_create_external_buffer finalizer timing
+// See: https://github.com/oven-sh/bun/issues/26446
+nativeTests.test_issue_26446 = async () => {
+  // Create buffer and verify data integrity
+  let buffer = nativeTests.test_napi_external_buffer_finalizer();
+
+  if (!buffer) {
+    console.log("FAIL: Could not create external buffer");
+    return;
+  }
+
+  // Verify we can read the buffer data
+  {
+    const data = new Uint8Array(buffer.buffer || buffer);
+    let dataValid = true;
+    for (let i = 0; i < data.length; i++) {
+      if (data[i] !== (i & 0xff)) {
+        console.log(`FAIL: Data corruption at offset ${i}: expected ${i & 0xff}, got ${data[i]}`);
+        dataValid = false;
+        break;
+      }
+    }
+
+    if (dataValid) {
+      console.log("PASS: Buffer data is correct after creation");
+    }
+  }
+  // data goes out of scope here, allowing GC
+
+  // Clear the buffer reference to allow GC
+  buffer = null;
+
+  // Use gcUntil to deterministically wait for finalizer to be called
+  await gcUntil(() => nativeTests.test_napi_external_buffer_finalizer_check());
+
+  console.log("PASS: External buffer finalizer was called");
+};
+
 nativeTests.test_get_value_string = () => {
   function to16Bit(string) {
     if (typeof Bun != "object") return string;

test/napi/napi-app/standalone_tests.cpp

@@ -1825,6 +1825,99 @@ static napi_value test_napi_empty_buffer_info(const Napi::CallbackInfo &info) {
   return ok(env);
 }
 
+// Test for napi_create_external_buffer finalizer timing
+// See: https://github.com/oven-sh/bun/issues/26446
+// The finalizer must be tied directly to the ArrayBuffer lifecycle to prevent
+// native code from freeing/reusing memory while JS still holds a reference.
+static bool g_external_buffer_finalizer_called = false;
+static void *g_external_buffer_finalized_data = nullptr;
+
+static void external_buffer_timing_finalizer(napi_env env, void *data,
+                                              void *hint) {
+  g_external_buffer_finalizer_called = true;
+  g_external_buffer_finalized_data = data;
+  // Free the data that was allocated in the test
+  free(data);
+}
+
+static napi_value
+test_napi_external_buffer_finalizer(const Napi::CallbackInfo &info) {
+  Napi::Env env = info.Env();
+
+  // Reset global state
+  g_external_buffer_finalizer_called = false;
+  g_external_buffer_finalized_data = nullptr;
+
+  // Allocate data that will be owned by the external buffer
+  const size_t data_size = 32 * 1024; // 32KB, larger than 16KB boundary
+  uint8_t *data = static_cast<uint8_t *>(malloc(data_size));
+  if (!data) {
+    printf("FAIL: Could not allocate test data\n");
+    return env.Undefined();
+  }
+
+  // Fill with a pattern we can verify
+  for (size_t i = 0; i < data_size; i++) {
+    data[i] = static_cast<uint8_t>(i & 0xFF);
+  }
+
+  // Create external buffer - the finalizer should be tied to the ArrayBuffer
+  napi_value buffer;
+  napi_status status = napi_create_external_buffer(
+      env, data_size, data, external_buffer_timing_finalizer, nullptr, &buffer);
+
+  if (status != napi_ok) {
+    printf("FAIL: napi_create_external_buffer failed with status %d\n", status);
+    free(data);
+    return env.Undefined();
+  }
+
+  // Verify we can read the data correctly
+  void *buffer_data;
+  size_t buffer_length;
+  status = napi_get_buffer_info(env, buffer, &buffer_data, &buffer_length);
+
+  if (status != napi_ok) {
+    printf("FAIL: napi_get_buffer_info failed with status %d\n", status);
+    return env.Undefined();
+  }
+
+  if (buffer_length != data_size) {
+    printf("FAIL: Buffer length is %zu, expected %zu\n", buffer_length,
+           data_size);
+    return env.Undefined();
+  }
+
+  if (buffer_data != data) {
+    printf("FAIL: Buffer data pointer does not match original data\n");
+    return env.Undefined();
+  }
+
+  // Verify the data content is correct (not corrupted)
+  uint8_t *verify_data = static_cast<uint8_t *>(buffer_data);
+  for (size_t i = 0; i < data_size; i++) {
+    if (verify_data[i] != static_cast<uint8_t>(i & 0xFF)) {
+      printf("FAIL: Data corruption at offset %zu: expected %u, got %u\n", i,
+             static_cast<unsigned>(i & 0xFF),
+             static_cast<unsigned>(verify_data[i]));
+      return env.Undefined();
+    }
+  }
+
+  // Return the buffer for JavaScript to hold
+  // The finalizer should NOT be called until this buffer is garbage collected
+  printf("PASS: napi_create_external_buffer created buffer with correct data\n");
+  return buffer;
+}
+
+static napi_value
+test_napi_external_buffer_finalizer_check(const Napi::CallbackInfo &info) {
+  Napi::Env env = info.Env();
+
+  // Return boolean indicating whether finalizer was called
+  return Napi::Boolean::New(env, g_external_buffer_finalizer_called);
+}
+
 // Test for napi_typeof with boxed primitive objects (String, Number, Boolean)
 // See: https://github.com/oven-sh/bun/issues/25351
 static napi_value napi_get_typeof(const Napi::CallbackInfo &info) {
@@ -1887,6 +1980,8 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) {
   REGISTER_FUNCTION(env, exports, test_napi_freeze_seal_indexed);
   REGISTER_FUNCTION(env, exports, test_napi_create_external_buffer_empty);
   REGISTER_FUNCTION(env, exports, test_napi_empty_buffer_info);
+  REGISTER_FUNCTION(env, exports, test_napi_external_buffer_finalizer);
+  REGISTER_FUNCTION(env, exports, test_napi_external_buffer_finalizer_check);
   REGISTER_FUNCTION(env, exports, napi_get_typeof);
 }
 

test/napi/napi.test.ts

@@ -282,6 +282,14 @@ describe.concurrent("napi", () => {
       expect(result).toContain("PASS: napi_is_detached_arraybuffer returns true for empty buffer's arraybuffer");
       expect(result).not.toContain("FAIL");
     });
+
+    // https://github.com/oven-sh/bun/issues/26446
+    it("calls finalizer when buffer is garbage collected", async () => {
+      const result = await checkSameOutput("test_issue_26446", []);
+      expect(result).toContain("PASS: napi_create_external_buffer created buffer with correct data");
+      expect(result).toContain("PASS: Buffer data is correct after creation");
+      expect(result).not.toContain("FAIL");
+    });
   });
 
   describe("napi_async_work", () => {