ffi: throw on fast calls to a closed library

doc/api/ffi.md

@@ -322,7 +322,8 @@ Calling `library.close()` (or disposing the library) more than once is a no-op.
 
 After a library has been closed:
 
-* Resolved function wrappers become invalid.
+* Resolved function wrappers become invalid. Calling one throws
+  [`ERR_FFI_LIBRARY_CLOSED`][], including on optimized call sites.
 * Further symbol and function resolution throws.
 * Registered callbacks are invalidated.
 
@@ -757,6 +758,7 @@ and keep callback and pointer lifetimes explicit on the native side.
 
 [Permission Model]: permissions.md#permission-model
 [`--allow-ffi`]: cli.md#--allow-ffi
+[`ERR_FFI_LIBRARY_CLOSED`]: errors.md#err_ffi_library_closed
 [`ffi.toBuffer(pointer, length, copy)`]: #ffitobufferpointer-length-copy
 [`using`]: https://tc39.es/proposal-explicit-resource-management/#sec-using-declarations
 [type names]: #type-names

lib/internal/ffi/fast-api.js

@@ -2,13 +2,16 @@
 
 const {
   ArrayPrototypeIncludes,
+  Error,
   NumberIsInteger,
   ObjectDefineProperty,
   ReflectApply,
   StringPrototypeIncludes,
   TypeError,
 } = primordials;
 
+const assert = require('internal/assert');
+
 const {
   Buffer,
 } = require('buffer');
@@ -21,6 +24,7 @@ const {
 const {
   charIsSigned,
   getRawPointer,
+  kClosedFlag,
   kFastArguments,
   kFastBufferInvoke,
 } = internalBinding('ffi');
@@ -68,6 +72,31 @@ function throwFFIArgCountError(expected, actual) {
     `Invalid argument count: expected ${expected}, got ${actual}`);
 }
 
+// Mirrors THROW_ERR_FFI_LIBRARY_CLOSED in src/node_errors.h. The native
+// `closed` checks in InvokeFunction/InvokeFunctionSB cannot run on the fast-call
+// path: V8 embeds the trampoline address in optimized code, so a tiered-up call
+// site jumps straight into the library that `close()` unloaded.
+function throwLibraryClosed() {
+  // eslint-disable-next-line no-restricted-syntax
+  const err = new Error('Library is closed');
+  err.code = 'ERR_FFI_LIBRARY_CLOSED';
+  throw err;
+}
+
+// Guard-only wrappers for fast signatures that need no argument conversion.
+// The flag is a one-byte Uint8Array, so the check is an element load that V8
+// cannot fold away across the call.
+function wrapWithClosedGuard(rawFn, nargs, closedFlag) {
+  function wrapper(...args) {
+    if (args.length !== nargs) {
+      throwFFIArgCountError(nargs, args.length);
+    }
+    if (closedFlag[0] !== 0) throwLibraryClosed();
+    return ReflectApply(rawFn, undefined, args);
+  }
+  return inheritMetadata(wrapper, rawFn, nargs);
+}
+
 function validateFastIntegerArg(type, value, index) {
   const info = fastIntegerTypeInfo[type];
   if (info === undefined) return;
@@ -202,7 +231,7 @@ function inheritMetadata(wrapper, rawFn, nargs) {
   return wrapper;
 }
 
-function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
+function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
   if (rawFn === undefined || rawFn === null) {
     return rawFn;
   }
@@ -213,9 +242,19 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
     return rawFn;
   }
 
+  // `kFastArguments` is attached by `CreateFunction` to every function that got
+  // a fast-call trampoline, so reaching here means the raw function must never
+  // be exposed unguarded: after `close()` the trampoline still points into the
+  // unloaded library and only a JavaScript-side check can stop the call.
+  // `DynamicLibrary::New` installs the flag on every instance, so a missing one
+  // means the native side and this wrapper are out of sync.
+  const closedFlag = owner?.[kClosedFlag];
+  assert(closedFlag !== undefined,
+         'FFI: fast function is missing the owning library closed flag');
+
   const indexes = getFastArgumentIndexes(argumentTypes);
   if (indexes === null) {
-    return rawFn;
+    return wrapWithClosedGuard(rawFn, argumentTypes.length, closedFlag);
   }
 
   const stringState = {
@@ -236,6 +275,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
       if (arguments.length !== 1) {
         throwFFIArgCountError(1, arguments.length);
       }
+      if (closedFlag[0] !== 0) throwLibraryClosed();
       validateFastIntegerArg(t0, a0, 0);
       let arg = a0;
       if (needsNullPointerConversion(t0) &&
@@ -265,6 +305,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
       if (arguments.length !== 2) {
         throwFFIArgCountError(2, arguments.length);
       }
+      if (closedFlag[0] !== 0) throwLibraryClosed();
       const stringCall = (c0 && hasStringPointerArg(t0, a0)) ||
                          (c1 && hasStringPointerArg(t1, a1));
       if (stringCall) enterStringConversion(stringState);
@@ -286,6 +327,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
       if (arguments.length !== 3) {
         throwFFIArgCountError(3, arguments.length);
       }
+      if (closedFlag[0] !== 0) throwLibraryClosed();
       const stringCall = (c0 && hasStringPointerArg(t0, a0)) ||
                          (c1 && hasStringPointerArg(t1, a1)) ||
                          (c2 && hasStringPointerArg(t2, a2));
@@ -303,6 +345,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, _owner) {
       if (args.length !== nargs) {
         throwFFIArgCountError(nargs, args.length);
       }
+      if (closedFlag[0] !== 0) throwLibraryClosed();
       let stringCall = false;
       for (let i = 0; i < indexes.length; i++) {
         const index = indexes[i];

src/env_properties.h

@@ -52,6 +52,7 @@
   V(ffi_sb_return_symbol, "ffi_sb_return_symbol")                              \
   V(ffi_fast_arguments_symbol, "ffi_fast_arguments_symbol")                    \
   V(ffi_fast_buffer_invoke_symbol, "ffi_fast_buffer_invoke_symbol")            \
+  V(ffi_closed_flag_symbol, "ffi_closed_flag_symbol")                          \
   V(constructor_key_symbol, "constructor_key_symbol")                          \
   V(handle_onclose_symbol, "handle_onclose")                                   \
   V(no_message_symbol, "no_message_symbol")                                    \

src/node_ffi.cc

@@ -38,6 +38,7 @@ using v8::PropertyAttribute;
 using v8::ReadOnly;
 using v8::String;
 using v8::TryCatch;
+using v8::Uint8Array;
 using v8::Value;
 
 namespace ffi {
@@ -72,6 +73,8 @@ void DynamicLibrary::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackFieldWithSize(
       "symbols", symbols_size, "std::unordered_map<std::string, void*>");
 
+  tracker->TrackField("closed_flag", closed_flag_);
+
   // FFIFunctionInfo instances and their sb_backing ArrayBuffers are
   // owned by V8 function wrappers and reachable only via weak references,
   // so they are deliberately not counted here.
@@ -83,6 +86,16 @@ void DynamicLibrary::Close() {
     fn->ptr = nullptr;
   }
 
+  // Generated fast-call trampolines are not torn down here. V8 bakes the
+  // trampoline address into optimized code, so releasing it while a compiled
+  // call site still holds it would replace a call into the unloaded library
+  // with a call into unmapped memory. The trampoline stays mapped for the
+  // lifetime of its FFIFunctionInfo and is instead kept unreachable by the
+  // JavaScript-side check on this flag.
+  if (closed_flag_ != nullptr) {
+    *static_cast<uint8_t*>(closed_flag_->Data()) = 1;
+  }
+
   // Closing the library invalidates all registered callbacks. Node.js does not
   // track or revoke callback pointers that have already been handed to native
   // code. If native code calls a callback pointer after `close()` or
@@ -383,10 +396,16 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
     }
   }
 
-  if (needs_fast_argument_wrapper || needs_fast_buffer_invoke) {
+  if (use_fast_api) {
     // Fast API wrappers need only the parameter type names. Result conversion
     // is still handled by V8's CFunction metadata, unlike the SharedBuffer path
     // which must also know how to read slot 0.
+    //
+    // These names are attached to every fast function, not just the ones that
+    // need `needs_fast_argument_wrapper` or `needs_fast_buffer_invoke`
+    // conversions, because their presence is also how the JavaScript side
+    // recognizes a function that can reach a trampoline and therefore needs the
+    // closed-library guard.
     Local<Value> arguments_arr;
     if (!ToV8Value(context, fn->arg_type_names, isolate)
              .ToLocal(&arguments_arr)) {
@@ -479,6 +498,21 @@ void DynamicLibrary::New(const FunctionCallbackInfo<Value>& args) {
     THROW_ERR_FFI_CALL_FAILED(env, "dlopen failed: %s", uv_dlerror(&lib->lib_));
     return;
   }
+
+  // Publish the closed flag on the instance for the JS-side fast-call guard.
+  // The backing store is retained so `Close()` can still write the flag after
+  // the view is collected, and so it outlives any wrapper closure reading it.
+  Local<ArrayBuffer> flag = ArrayBuffer::New(env->isolate(), 1);
+  lib->closed_flag_ = flag->GetBackingStore();
+  if (!args.This()
+           ->DefineOwnProperty(
+               env->context(),
+               env->ffi_closed_flag_symbol(),
+               Uint8Array::New(flag, 0, 1),
+               static_cast<PropertyAttribute>(ReadOnly | DontEnum | DontDelete))
+           .FromMaybe(false)) {
+    lib->Close();
+  }
 }
 
 void DynamicLibrary::Close(const FunctionCallbackInfo<Value>& args) {
@@ -1323,6 +1357,13 @@ static void Initialize(Local<Object> target,
             FIXED_ONE_BYTE_STRING(isolate, "kFastBufferInvoke"),
             env->ffi_fast_buffer_invoke_symbol())
       .Check();
+  // Keys the per-library closed flag that fast wrappers test before calling a
+  // generated trampoline.
+  target
+      ->Set(context,
+            FIXED_ONE_BYTE_STRING(isolate, "kClosedFlag"),
+            env->ffi_closed_flag_symbol())
+      .Check();
 }
 
 }  // namespace ffi

src/node_ffi.h

@@ -145,6 +145,11 @@ class DynamicLibrary : public BaseObject {
   bool is_closed() const;
 
   uv_lib_t lib_ = {};
+  // Single-byte flag shared with JavaScript, set to 1 by `Close()`. The
+  // `closed` checks in the C++ invokers cannot run once V8 has compiled a
+  // fast-call site, because the trampoline address is embedded in the
+  // optimized code, so the JS wrapper tests this flag before entering it.
+  std::shared_ptr<v8::BackingStore> closed_flag_;
   std::string path_;
   std::unordered_map<std::string, void*> symbols_;
   std::unordered_map<std::string, std::shared_ptr<FFIFunction>> functions_;

test/ffi/test-ffi-closed-fast-calls.js

@@ -0,0 +1,119 @@
+// Flags: --experimental-ffi --allow-natives-syntax
+'use strict';
+
+// Regression test for calls that reach a generated fast-call trampoline after
+// the owning library has been closed.
+//
+// `DynamicLibrary::Close()` sets `FFIFunction::closed` and clears
+// `FFIFunction::ptr`, and both C++ invokers check them. Neither check can run on
+// the fast-call path: the trampoline is created with the resolved symbol address
+// and V8 embeds the trampoline address in optimized code, so a call site that
+// has tiered up jumps straight into the unloaded library.
+
+const common = require('../common');
+
+common.skipIfFFIMissing();
+
+const assert = require('node:assert');
+const { test } = require('node:test');
+const ffi = require('node:ffi');
+const { cString, fixtureSymbols, libraryPath } = require('./ffi-test-common');
+
+// Every function that has a trampoline is exposed as a JavaScript wrapper, and
+// that wrapper holds the fast-call site, so it is the function that has to be
+// optimized to put the trampoline in play. Optimizing only an outer caller does
+// not reach it, and neither does calling after `close()`: a call site that
+// always throws never tiers up, which is why a closed library used to survive a
+// single call and then crash under sustained use.
+function optimizeFastCall(fn, args) {
+  assert.doesNotMatch(fn.toString(), /\[native code\]/,
+                      'FFI functions with a fast-call trampoline must be ' +
+                      'exposed as JavaScript wrappers');
+  eval('%PrepareFunctionForOptimization(fn)');
+  fn(...args);
+  fn(...args);
+  eval('%OptimizeFunctionOnNextCall(fn)');
+  fn(...args);
+}
+
+// The guard must hold for repeated calls, not just the first one.
+function assertAlwaysClosed(fn, args) {
+  assert.throws(() => fn(...args), { code: 'ERR_FFI_LIBRARY_CLOSED' });
+
+  for (let i = 0; i < 1000; i++) {
+    let code;
+    try {
+      fn(...args);
+    } catch (err) {
+      code = err.code;
+    }
+    assert.strictEqual(code, 'ERR_FFI_LIBRARY_CLOSED');
+  }
+}
+
+test('closed library throws from optimized scalar fast calls', () => {
+  const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols);
+  const multiply = functions.multiply_f64;
+
+  optimizeFastCall(multiply, [2, 3]);
+  assert.strictEqual(multiply(2, 3), 6);
+
+  lib.close();
+  assertAlwaysClosed(multiply, [2, 3]);
+});
+
+test('closed library throws from optimized integer fast calls', () => {
+  const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols);
+  const add = functions.add_i32;
+
+  optimizeFastCall(add, [20, 22]);
+  assert.strictEqual(add(20, 22), 42);
+
+  lib.close();
+  assertAlwaysClosed(add, [20, 22]);
+});
+
+test('closed library throws from optimized pointer fast calls', () => {
+  const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols);
+  const strlen = functions.safe_strlen;
+  const buffer = cString('closed');
+
+  optimizeFastCall(strlen, [buffer]);
+  assert.strictEqual(strlen(buffer), 6);
+
+  lib.close();
+  assertAlwaysClosed(strlen, [buffer]);
+});
+
+test('closed library throws from optimized multi-argument fast calls', () => {
+  const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols);
+  const sum = functions.sum_five_f64;
+
+  optimizeFastCall(sum, [1, 2, 3, 4, 5]);
+  assert.strictEqual(sum(1, 2, 3, 4, 5), 15);
+
+  lib.close();
+  assertAlwaysClosed(sum, [1, 2, 3, 4, 5]);
+});
+
+test('closed library throws for functions from getFunction()', () => {
+  const lib = new ffi.DynamicLibrary(libraryPath);
+  const multiply = lib.getFunction('multiply_f64', fixtureSymbols.multiply_f64);
+
+  optimizeFastCall(multiply, [2, 3]);
+  assert.strictEqual(multiply(2, 3), 6);
+
+  lib.close();
+  assertAlwaysClosed(multiply, [2, 3]);
+});
+
+test('disposing a library invalidates retained fast wrappers', () => {
+  let multiply;
+  {
+    using handle = ffi.dlopen(libraryPath, fixtureSymbols);
+    multiply = handle.functions.multiply_f64;
+    optimizeFastCall(multiply, [2, 3]);
+  }
+
+  assertAlwaysClosed(multiply, [2, 3]);
+});