ffi: preserve strings during reentrant calls

Cache temporary string conversion buffers by wrapper and active call
depth. This prevents nested FFI calls from overwriting or replacing
buffers still in use by an outer native call.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.6-sol
PR-URL: #64551
Fixes: #64550
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: trivikr

lib/internal/ffi/fast-api.js

@@ -28,7 +28,6 @@ const {
 } = internalBinding('ffi');
 
 const kFastBuffer = Symbol('kFastBuffer');
-const kStringConversionBuffer = Symbol('kStringConversionBuffer');
 
 const U64_MAX = 0xFFFFFFFFFFFFFFFFn;
 const I64_MAX = 0x7FFFFFFFFFFFFFFFn;
@@ -119,19 +118,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;
@@ -157,13 +157,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);
@@ -189,10 +189,10 @@ function getFastArgumentIndexes(argumentsTypes, rawFn) {
   return indexes;
 }
 
-function convertFastArg(type, value, rawFn, owner, index) {
+function convertFastArg(type, value, rawFn, stringState, index) {
   validateFastIntegerArg(type, value, index);
   return needsPointerConversion(type, rawFn) ?
-    convertPointerArg(type, value, owner, index) : value;
+    convertPointerArg(type, value, stringState, index) : value;
 }
 
 function initializeFastBufferMetadata(rawFn, argumentTypes) {
@@ -228,7 +228,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;
   }
@@ -244,6 +244,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) {
@@ -262,7 +268,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);
@@ -280,8 +291,16 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
       if (arguments.length !== 2) {
         throwFFIArgCountError(2, arguments.length);
       }
-      return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0,
-                   c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1);
+      const stringCall = (c0 && hasStringPointerArg(t0, a0)) ||
+                         (c1 && hasStringPointerArg(t1, a1));
+      if (stringCall) enterStringConversion(stringState);
+      try {
+        return rawFn(c0 ?
+          convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
+                     c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1);
+      } finally {
+        if (stringCall) exitStringConversion(stringState);
+      }
     };
   } else if (nargs === 3) {
     const c0 = ArrayPrototypeIncludes(indexes, 0);
@@ -294,21 +313,43 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) {
       if (arguments.length !== 3) {
         throwFFIArgCountError(3, arguments.length);
       }
-      return rawFn(c0 ? convertFastArg(t0, a0, rawFn, owner, 0) : a0,
-                   c1 ? convertFastArg(t1, a1, rawFn, owner, 1) : a1,
-                   c2 ? convertFastArg(t2, a2, rawFn, 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 ?
+          convertFastArg(t0, a0, rawFn, stringState, 0) : a0,
+                     c1 ? convertFastArg(t1, a1, rawFn, stringState, 1) : a1,
+                     c2 ? convertFastArg(t2, a2, rawFn, 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] = convertFastArg(
-          argumentTypes[index], args[index], rawFn, 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] = convertFastArg(
+            argumentTypes[index], args[index], rawFn, stringState, index);
+        }
+        return ReflectApply(rawFn, undefined, args);
+      } finally {
+        if (stringCall) exitStringConversion(stringState);
       }
-      return ReflectApply(rawFn, undefined, args);
     };
   }
 

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);

test/ffi/test-ffi-fast-buffer.js

@@ -69,3 +69,29 @@ test('fast FFI buffer arguments reject invalid values', () => {
     lib.close();
   }
 });
+
+test('fast FFI string buffers survive reentrant callbacks', {
+  // Bundled libffi callbacks crash on SmartOS.
+  skip: common.isSunOS,
+}, () => {
+  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();
+  }
+});