diff --git a/lib/internal/ffi/fast-api.js b/lib/internal/ffi/fast-api.js index 15b13af23ceaa1..57dad51a34c037 100644 --- a/lib/internal/ffi/fast-api.js +++ b/lib/internal/ffi/fast-api.js @@ -26,7 +26,6 @@ const { } = internalBinding('ffi'); const kFastBuffer = Symbol('kFastBuffer'); -const kStringConversionBuffer = Symbol('kStringConversionBuffer'); function throwFFIArgError(msg) { // eslint-disable-next-line no-restricted-syntax @@ -79,19 +78,20 @@ function hasPointerMemoryArg(type, value) { (isArrayBufferView(value) || isAnyArrayBuffer(value)); } -function getStringConversionPointer(owner, value, index) { - const size = value.length * 3 + 1; - let buffers = owner[kStringConversionBuffer]; - if (buffers === undefined) { - buffers = []; - ObjectDefineProperty(owner, kStringConversionBuffer, { - __proto__: null, - configurable: false, - enumerable: false, - writable: false, - value: buffers, - }); +function enterStringConversion(state) { + if (state.buffers[state.depth] === undefined) { + state.buffers[state.depth] = []; } + state.depth++; +} + +function exitStringConversion(state) { + state.depth--; +} + +function getStringConversionPointer(state, value, index) { + const size = value.length * 3 + 1; + const buffers = state.buffers[state.depth - 1]; let entry = buffers[index]; if (entry !== undefined && entry.string === value) { return entry.pointer; @@ -117,13 +117,13 @@ function getStringConversionPointer(owner, value, index) { return entry.pointer; } -function convertPointerArg(type, value, owner, index) { +function convertPointerArg(type, value, stringState, index) { if (needsNullPointerConversion(type) && (value === null || value === undefined)) { return 0n; } if (hasStringPointerArg(type, value)) { - return getStringConversionPointer(owner, value, index); + return getStringConversionPointer(stringState, value, index); } if (hasPointerMemoryArg(type, value)) { return getRawPointer(value); @@ -181,7 +181,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; } @@ -197,6 +197,12 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { return rawFn; } + const stringState = { + __proto__: null, + buffers: [], + depth: 0, + }; + const nargs = argumentTypes.length; let wrapper; if (nargs === 1 && indexes.length === 1 && indexes[0] === 0) { @@ -214,7 +220,12 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { (arg === null || arg === undefined)) { arg = 0n; } else if (string0 && typeof arg === 'string') { - arg = getStringConversionPointer(owner, arg, 0); + enterStringConversion(stringState); + try { + return rawFn(getStringConversionPointer(stringState, arg, 0)); + } finally { + exitStringConversion(stringState); + } } else if (memory0 && (isArrayBufferView(arg) || isAnyArrayBuffer(arg))) { if (fastBufferInvoke !== undefined) { return fastBufferInvoke(arg); @@ -232,8 +243,15 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { if (arguments.length !== 2) { throwFFIArgCountError(2, arguments.length); } - return rawFn(c0 ? convertPointerArg(t0, a0, owner, 0) : a0, - c1 ? convertPointerArg(t1, a1, owner, 1) : a1); + const stringCall = (c0 && hasStringPointerArg(t0, a0)) || + (c1 && hasStringPointerArg(t1, a1)); + if (stringCall) enterStringConversion(stringState); + try { + return rawFn(c0 ? convertPointerArg(t0, a0, stringState, 0) : a0, + c1 ? convertPointerArg(t1, a1, stringState, 1) : a1); + } finally { + if (stringCall) exitStringConversion(stringState); + } }; } else if (nargs === 3) { const c0 = ArrayPrototypeIncludes(indexes, 0); @@ -246,21 +264,42 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { if (arguments.length !== 3) { throwFFIArgCountError(3, arguments.length); } - return rawFn(c0 ? convertPointerArg(t0, a0, owner, 0) : a0, - c1 ? convertPointerArg(t1, a1, owner, 1) : a1, - c2 ? convertPointerArg(t2, a2, owner, 2) : a2); + const stringCall = (c0 && hasStringPointerArg(t0, a0)) || + (c1 && hasStringPointerArg(t1, a1)) || + (c2 && hasStringPointerArg(t2, a2)); + if (stringCall) enterStringConversion(stringState); + try { + return rawFn(c0 ? convertPointerArg(t0, a0, stringState, 0) : a0, + c1 ? convertPointerArg(t1, a1, stringState, 1) : a1, + c2 ? convertPointerArg(t2, a2, stringState, 2) : a2); + } finally { + if (stringCall) exitStringConversion(stringState); + } }; } else { wrapper = function(...args) { if (args.length !== nargs) { throwFFIArgCountError(nargs, args.length); } + let stringCall = false; for (let i = 0; i < indexes.length; i++) { const index = indexes[i]; - args[index] = convertPointerArg( - argumentTypes[index], args[index], owner, index); + if (hasStringPointerArg(argumentTypes[index], args[index])) { + stringCall = true; + break; + } + } + if (stringCall) enterStringConversion(stringState); + try { + for (let i = 0; i < indexes.length; i++) { + const index = indexes[i]; + args[index] = convertPointerArg( + argumentTypes[index], args[index], stringState, index); + } + return ReflectApply(rawFn, undefined, args); + } finally { + if (stringCall) exitStringConversion(stringState); } - return ReflectApply(rawFn, undefined, args); }; } diff --git a/test/ffi/fixture_library/ffi_test_library.c b/test/ffi/fixture_library/ffi_test_library.c index c58a6536469dc8..10d2ed9f66c95d 100644 --- a/test/ffi/fixture_library/ffi_test_library.c +++ b/test/ffi/fixture_library/ffi_test_library.c @@ -333,6 +333,15 @@ FFI_EXPORT void call_void_callback(VoidCallback callback) { } } +FFI_EXPORT int32_t string_survives_callback(const char* str, + VoidCallback callback) { + if (callback) { + callback(); + } + + return str && strcmp(str, "outer string") == 0; +} + FFI_EXPORT void call_string_callback(StringCallback callback, const char* str) { if (callback) { callback(str); diff --git a/test/ffi/test-ffi-fast-buffer.js b/test/ffi/test-ffi-fast-buffer.js index ceca89b7c43285..f869a9615819f5 100644 --- a/test/ffi/test-ffi-fast-buffer.js +++ b/test/ffi/test-ffi-fast-buffer.js @@ -69,3 +69,26 @@ test('fast FFI buffer arguments reject invalid values', () => { lib.close(); } }); + +test('fast FFI string buffers survive reentrant callbacks', () => { + const { lib, functions } = ffi.dlopen(libraryPath, { + safe_strlen: { arguments: ['string'], return: 'i32' }, + string_survives_callback: { + arguments: ['string', 'pointer'], + return: 'i32', + }, + }); + let nestedLength; + const callback = lib.registerCallback(() => { + nestedLength = functions.safe_strlen('inner string'); + }); + + try { + assert.strictEqual( + functions.string_survives_callback('outer string', callback), 1); + assert.strictEqual(nestedLength, 12); + } finally { + lib.unregisterCallback(callback); + lib.close(); + } +});