ffi: keep DynamicLibrary alive while callbacks are registered

doc/api/ffi.md

@@ -482,6 +482,12 @@ When `signature` is omitted, the callback uses a default `void ()` signature.
 The return value is the callback pointer address as a `bigint`. It can be
 passed to native functions expecting a callback pointer.
 
+While a library has registered callbacks, it is not garbage collected, even if
+no JavaScript references to it remain. This keeps callback pointers held by
+native code valid until `library.unregisterCallback(pointer)` or
+`library.close()` is called. Unregister callbacks or close the library when
+they are no longer needed to avoid leaking it.
+
 ```cjs
 const { DynamicLibrary, suffix } = require('node:ffi');
 

src/node_ffi.cc

@@ -119,6 +119,15 @@ void DynamicLibrary::Close() {
   function_wrappers_.clear();
   callbacks_.clear();
 }
+void DynamicLibrary::UpdateCallbackRetention() {
+  // Must not be called from ~DynamicLibrary(): the persistent handle may
+  // already have been reset by the weak callback.
+  if (callbacks_.empty()) {
+    MakeWeak();
+  } else {
+    ClearWeak();
+  }
+}
 
 Maybe<void*> DynamicLibrary::ResolveSymbol(Environment* env,
                                            const std::string& name) {
@@ -585,6 +594,7 @@ void DynamicLibrary::Close(const FunctionCallbackInfo<Value>& args) {
   // Closing a library from one of its active callbacks is unsupported and
   // dangerous. Callbacks must return before the owning library is closed.
   lib->Close();
+  lib->UpdateCallbackRetention();
 }
 
 void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo<Value>& args) {
@@ -1176,6 +1186,7 @@ void DynamicLibrary::RegisterCallback(const FunctionCallbackInfo<Value>& args) {
 
   auto ret = static_cast<uint64_t>(reinterpret_cast<uintptr_t>(callback->ptr));
   lib->callbacks_.emplace(callback->ptr, std::move(callback));
+  lib->UpdateCallbackRetention();
   args.GetReturnValue().Set(BigInt::NewFromUnsigned(isolate, ret));
 }
 
@@ -1214,6 +1225,7 @@ void DynamicLibrary::UnregisterCallback(
   // output, or corrupt memory. Unregistering a callback while it is currently
   // executing is also unsupported and dangerous.
   lib->callbacks_.erase(existing);
+  lib->UpdateCallbackRetention();
 }
 
 void DynamicLibrary::RefCallback(const FunctionCallbackInfo<Value>& args) {

src/node_ffi.h

@@ -148,6 +148,10 @@ class DynamicLibrary : public BaseObject {
 
  private:
   void Close();
+  // Keeps the JS object strong while callbacks are registered. Callback
+  // pointers are plain addresses that cannot root the library, so without
+  // this a GC could free trampolines that native code still holds.
+  void UpdateCallbackRetention();
   v8::Maybe<void*> ResolveSymbol(Environment* env, const std::string& name);
   struct PreparedFunction {
     std::shared_ptr<FFIFunction> fn;

test/ffi/test-ffi-callback-gc.js

@@ -0,0 +1,77 @@
+// Flags: --expose-gc
+'use strict';
+const { skipIfFFIMissing } = require('../common');
+skipIfFFIMissing();
+
+const { gcUntil } = require('../common/gc');
+const test = require('node:test');
+const ffi = require('node:ffi');
+const { fixtureSymbols, libraryPath } = require('./ffi-test-common');
+
+const signature = { arguments: ['i32'], return: 'i32' };
+
+test('registered callbacks keep an unreachable library alive', async (t) => {
+  // A separate handle provides the native function that invokes the callback.
+  const { lib: keeper, functions: symbols } =
+    ffi.dlopen(libraryPath, fixtureSymbols);
+  t.after(() => keeper.close());
+
+  let lib = new ffi.DynamicLibrary(libraryPath);
+  const ref = new WeakRef(lib);
+  const pointer = lib.registerCallback(signature, (x) => x * 2);
+  t.assert.strictEqual(symbols.call_int_callback(pointer, 21), 42);
+
+  lib = null;
+  for (let i = 0; i < 5; i++) {
+    await gcUntil('library with callbacks stays alive', () => true, 1);
+    t.assert.strictEqual(typeof ref.deref(), 'object');
+    t.assert.strictEqual(symbols.call_int_callback(pointer, 21), 42);
+  }
+
+  ref.deref().unregisterCallback(pointer);
+});
+
+test('library becomes collectable after its callbacks are unregistered', async () => {
+  let lib = new ffi.DynamicLibrary(libraryPath);
+  const ref = new WeakRef(lib);
+  const first = lib.registerCallback(signature, (x) => x);
+  const second = lib.registerCallback(signature, (x) => x);
+  lib.unregisterCallback(first);
+  lib.unregisterCallback(second);
+  lib = null;
+
+  await gcUntil('library is collected after unregisterCallback',
+                () => ref.deref() === undefined);
+});
+
+test('library becomes collectable after close()', async () => {
+  let lib = new ffi.DynamicLibrary(libraryPath);
+  const ref = new WeakRef(lib);
+  lib.registerCallback(signature, (x) => x);
+  lib.close();
+  lib = null;
+
+  await gcUntil('library is collected after close()',
+                () => ref.deref() === undefined);
+});
+
+test('unrefCallback() does not release the callback trampoline', async (t) => {
+  const { lib: keeper, functions: symbols } =
+    ffi.dlopen(libraryPath, fixtureSymbols);
+  t.after(() => keeper.close());
+
+  let lib = new ffi.DynamicLibrary(libraryPath);
+  const ref = new WeakRef(lib);
+  const pointer = lib.registerCallback(signature, () => 1);
+  lib.unrefCallback(pointer);
+  lib = null;
+
+  for (let i = 0; i < 5; i++) {
+    await gcUntil('library with unref callbacks stays alive', () => true, 1);
+    t.assert.strictEqual(typeof ref.deref(), 'object');
+  }
+  // The JS function may be gone, but the trampoline must still be callable.
+  symbols.call_int_callback(pointer, 21);
+
+  ref.deref().unregisterCallback(pointer);
+});