From 4812e87306dd9c09185bcc2f109e104fe8259dfa Mon Sep 17 00:00:00 2001
From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Date: Fri, 31 Jul 2026 20:02:28 +0000
Subject: [PATCH] ffi: fix crash in refCallback and unrefCallback

Both handlers called fn.ClearWeak() and fn.SetWeak() without checking
whether the persistent handle was still populated. After the callback
function is garbage collected following an earlier unrefCallback()
call, the handle is empty and both V8 methods dereference a null slot.

InvokeCallback() already guarded against this. Add the same check to
both handlers and throw ERR_INVALID_ARG_VALUE, matching the existing
behavior for a pointer that is not in the callback map.

Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5
---
 doc/api/ffi.md                     |  7 +++++++
 src/node_ffi.cc                    | 17 ++++++++++++++++
 test/ffi/test-ffi-weakref-calls.js | 31 ++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/doc/api/ffi.md b/doc/api/ffi.md
index 94dc9e42f03d..56d3329b6315 100644
--- a/doc/api/ffi.md
+++ b/doc/api/ffi.md
@@ -459,6 +459,10 @@ memory.
 
 Keeps the callback strongly referenced by JavaScript.
 
+Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been
+garbage collected after a previous `library.unrefCallback(pointer)` call, since
+a collected function cannot be referenced again.
+
 ### `library.unrefCallback(pointer)`
 
 * `pointer` {bigint}
@@ -469,6 +473,9 @@ If the callback function is later garbage collected, subsequent native
 invocations become a no-op. Non-void return values are zero-initialized before
 returning to native code.
 
+Throws `ERR_INVALID_ARG_VALUE` if the callback function has already been
+garbage collected.
+
 ## Calling native functions
 
 Argument conversion depends on the declared FFI type.
diff --git a/src/node_ffi.cc b/src/node_ffi.cc
index b05d09270126..4424ad6afbc6 100644
--- a/src/node_ffi.cc
+++ b/src/node_ffi.cc
@@ -1125,6 +1125,15 @@ void DynamicLibrary::RefCallback(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  // The callback function may already have been collected after a previous
+  // unrefCallback() call. The persistent handle is empty in that case, and
+  // ClearWeak() on an empty handle dereferences a null slot. There is also no
+  // function left to make strong again.
+  if (existing->second->fn.IsEmpty()) {
+    THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");
+    return;
+  }
+
   existing->second->fn.ClearWeak();
 }
 
@@ -1156,6 +1165,14 @@ void DynamicLibrary::UnrefCallback(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  // The callback function may already have been collected by a previous
+  // unrefCallback() call. The persistent handle is empty in that case, and
+  // SetWeak() on an empty handle dereferences a null slot.
+  if (existing->second->fn.IsEmpty()) {
+    THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");
+    return;
+  }
+
   existing->second->fn.SetWeak();
 }
 
diff --git a/test/ffi/test-ffi-weakref-calls.js b/test/ffi/test-ffi-weakref-calls.js
index 1a2a2eaeab87..b158ee810776 100644
--- a/test/ffi/test-ffi-weakref-calls.js
+++ b/test/ffi/test-ffi-weakref-calls.js
@@ -51,6 +51,37 @@ test('ffi refCallback retains callback function', async (t) => {
   lib.unregisterCallback(pointer);
 });
 
+test('callback ref/unref throw after callback function is collected', async (t) => {
+  const { lib } = ffi.dlopen(libraryPath, fixtureSymbols);
+  t.after(() => lib.close());
+
+  let callback = () => 1;
+  const ref = new WeakRef(callback);
+  const pointer = lib.registerCallback(
+    { arguments: ['i32'], return: 'i32' },
+    callback,
+  );
+
+  lib.unrefCallback(pointer);
+  callback = null;
+
+  await gcUntil(
+    'callback ref/unref throw after callback function is collected',
+    () => ref.deref() === undefined,
+  );
+
+  t.assert.throws(() => lib.unrefCallback(pointer), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: /Callback not found/,
+  });
+  t.assert.throws(() => lib.refCallback(pointer), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: /Callback not found/,
+  });
+
+  lib.unregisterCallback(pointer);
+});
+
 test('callback ref/unref/unregister throw when library is closed', (t) => {
   const { lib } = ffi.dlopen(libraryPath, fixtureSymbols);
   const callback = lib.registerCallback(() => {});