ffi: preserve strings during reentrant calls

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

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,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();
+  }
+});