diff --git a/src/ffi/fast.cc b/src/ffi/fast.cc index 4f6606d5025..aaba55e07ba 100644 --- a/src/ffi/fast.cc +++ b/src/ffi/fast.cc @@ -245,7 +245,13 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local value, // returns zero after throwing, preventing the native target from seeing an // invalid pointer value. constexpr uintptr_t kInvalidBuffer = std::numeric_limits::max(); - v8::Isolate* isolate = options != nullptr ? options->isolate : nullptr; + // The trampoline always passes the FastApiCallbackOptions declared in the + // CFunctionInfo. No HandleScope is active during a Fast API call, and both + // ArrayBufferView::Buffer() and the error paths below create handles, so + // open one for the whole helper. Otherwise every call leaks handles into + // the caller's scope. + v8::Isolate* isolate = options->isolate; + v8::HandleScope scope(isolate); // Accept only the memory-backed JS values supported by ToFFIArgument in the // native helper. Other pointer conversions, including strings and direct @@ -254,41 +260,26 @@ extern "C" uintptr_t node_ffi_fast_buffer_data(v8::Local value, if (value->IsArrayBufferView()) { v8::Local view = value.As(); if (view->Buffer()->WasDetached()) { - if (isolate != nullptr) { - // No HandleScope is active during a Fast API call, so open one before - // creating the error object. - v8::HandleScope scope(isolate); - THROW_ERR_INVALID_ARG_VALUE( - isolate, - "Argument %u is an ArrayBufferView backed by a detached " - "ArrayBuffer", - index); - } + THROW_ERR_INVALID_ARG_VALUE( + isolate, + "Argument %u is an ArrayBufferView backed by a detached " + "ArrayBuffer", + index); return kInvalidBuffer; } return PointerFromValue(value); } if (value->IsArrayBuffer()) { if (value.As()->WasDetached()) { - if (isolate != nullptr) { - // No HandleScope is active during a Fast API call, so open one before - // creating the error object. - v8::HandleScope scope(isolate); - THROW_ERR_INVALID_ARG_VALUE( - isolate, "Argument %u is a detached ArrayBuffer", index); - } + THROW_ERR_INVALID_ARG_VALUE( + isolate, "Argument %u is a detached ArrayBuffer", index); return kInvalidBuffer; } return PointerFromValue(value); } - if (isolate != nullptr) { - // No HandleScope is active during a Fast API call, so open one before - // creating the error object. - v8::HandleScope scope(isolate); - THROW_ERR_INVALID_ARG_VALUE( - isolate, "Argument %u must be a buffer or an ArrayBuffer", index); - } + THROW_ERR_INVALID_ARG_VALUE( + isolate, "Argument %u must be a buffer or an ArrayBuffer", index); return kInvalidBuffer; } diff --git a/test/ffi/test-ffi-fast-buffer.js b/test/ffi/test-ffi-fast-buffer.js index edf4a297aa4..946b7610249 100644 --- a/test/ffi/test-ffi-fast-buffer.js +++ b/test/ffi/test-ffi-fast-buffer.js @@ -6,6 +6,7 @@ common.skipIfFFIMissing(); const assert = require('node:assert'); const { test } = require('node:test'); +const { queryObjects } = require('node:v8'); const { internalBinding } = require('internal/test/binding'); const { kSbSharedBuffer, @@ -100,6 +101,36 @@ test('optimized pointer arguments reject direct SharedArrayBuffers', () => { } }); +test('optimized buffer calls do not retain handles', () => { + const { lib, functions } = ffi.dlopen(libraryPath, { + first_byte: { arguments: ['buffer'], return: 'u8' }, + }); + // A leaked handle to the backing store keeps it alive until the current + // synchronous run ends, so count live backing stores after the calls. + class TrackedArrayBuffer extends ArrayBuffer {} + + function callFirstByte(view) { + return functions.first_byte(view); + } + + try { + const warm = new Uint8Array(1); + eval('%PrepareFunctionForOptimization(callFirstByte)'); + for (let i = 0; i < 1000; i++) callFirstByte(warm); + eval('%OptimizeFunctionOnNextCall(callFirstByte)'); + callFirstByte(warm); + + for (let i = 0; i < 100; i++) { + callFirstByte(new Uint8Array(new TrackedArrayBuffer(1))); + } + assert.strictEqual( + queryObjects(TrackedArrayBuffer, { format: 'count' }), 0); + } finally { + eval('%WaitForBackgroundOptimization()'); + lib.close(); + } +}); + test('fast FFI string buffers survive reentrant callbacks', { // Bundled libffi callbacks crash on SmartOS. skip: common.isSunOS,