From aa0c8b55d4c6c74ab2d12e4f30c8366a439dd535 Mon Sep 17 00:00:00 2001 From: Soul Lee Date: Sun, 20 Sep 2026 16:20:36 +0900 Subject: [PATCH] ffi: allocate string argument storage lazily Reserve temporary string storage in InvokeFunction only when the first string argument is encountered. Calls without string arguments avoid allocating and freeing an unused vector buffer. Reserve capacity for all arguments before saving the first string pointer so subsequent strings cannot invalidate it. Add coverage for multiple strings, mixed Buffer arguments, and errors during argument conversion. Signed-off-by: Soul Lee --- src/node_ffi.cc | 5 +- test/ffi/test-ffi-invoke-string-storage.js | 57 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 test/ffi/test-ffi-invoke-string-storage.js diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 495e59ba5cc..732657a368e 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -602,7 +602,6 @@ void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo& args) { std::vector values(expected_args, 0); std::vector ffi_args(expected_args, nullptr); std::vector strings; - strings.reserve(expected_args); for (unsigned int i = 0; i < expected_args; i++) { FFIArgumentCategory res; @@ -624,6 +623,10 @@ void DynamicLibrary::InvokeFunction(const FunctionCallbackInfo& args) { return; } + if (strings.empty()) { + // Keep string pointers stable as subsequent arguments are converted. + strings.reserve(expected_args); + } strings.push_back(*str); values[i] = reinterpret_cast(strings.back().c_str()); ffi_args[i] = &values[i]; diff --git a/test/ffi/test-ffi-invoke-string-storage.js b/test/ffi/test-ffi-invoke-string-storage.js new file mode 100644 index 00000000000..c54e55b37d4 --- /dev/null +++ b/test/ffi/test-ffi-invoke-string-storage.js @@ -0,0 +1,57 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +common.skipIfFFIMissing(); + +const assert = require('node:assert'); +const { internalBinding } = require('internal/test/binding'); +const { DynamicLibrary, kSbInvokeSlow } = internalBinding('ffi'); +// Capture the native method before node:ffi installs argument conversions. +const getFunction = DynamicLibrary.prototype.getFunction; +const ffi = require('node:ffi'); +const { libraryPath, cString } = require('./ffi-test-common'); + +const lib = new ffi.DynamicLibrary(libraryPath); +const rawConcat = getFunction.call(lib, 'string_concat', { + arguments: ['pointer', 'pointer'], + return: 'pointer', +}); +// String and Buffer arguments cannot use the scalar Fast API entrypoint. +// On SharedBuffer platforms, select its native fallback explicitly. +const concat = rawConcat[kSbInvokeSlow] ?? rawConcat; +const free = lib.getFunction('free_string', { + arguments: ['pointer'], + return: 'void', +}); + +try { + for (const [left, right] of [ + ['', ''], + ['hello ', 'world'], + ['a'.repeat(128), 'b'.repeat(128)], + ['\u03b1', '\u03b2'], + ]) { + for (const [a, b] of [ + [left, right], + [cString(left), right], + [left, cString(right)], + [cString(left), cString(right)], + ]) { + const result = concat(a, b); + try { + assert.strictEqual(ffi.toString(result), left + right); + } finally { + free(result); + } + } + } + assert.throws(() => concat('hello', 'bad\0string'), { + code: 'ERR_INVALID_ARG_VALUE', + }); + assert.throws(() => concat('hello', 42), { + code: 'ERR_INVALID_ARG_VALUE', + }); +} finally { + lib.close(); +}