fix: suppress JS accessor callback reentry

DjDeveloperr · DjDeveloperr · commit 3743381d0f84 · 2026-06-30T04:34:35.000-04:00
diff --git a/NativeScript/ffi/shared/bridge/Callbacks.mm b/NativeScript/ffi/shared/bridge/Callbacks.mm
@@ -1459,6 +1459,7 @@ bool shouldSkipMethodCallback(void* args[], void* ret) {
     for (const auto& key :
          methodPolicy_.skipCallbackIfAssociatedObjectTruthy) {
       if (associatedObjectIsTruthy(receiver, key)) {
+        zeroReturnValue(ret);
         applyMethodPolicyAssignments(args);
         storePrimitivePolicyReturnValue(ret);
         return true;
@@ -1469,6 +1470,7 @@ bool shouldSkipMethodCallback(void* args[], void* ret) {
       return false;
     }
 
+    zeroReturnValue(ret);
     applyMethodPolicyAssignments(args);
     storePrimitivePolicyReturnValue(ret);
     return true;
diff --git a/NativeScript/ffi/shared/bridge/ClassBuilder.mm b/NativeScript/ffi/shared/bridge/ClassBuilder.mm
@@ -376,6 +376,13 @@ NativeApiMethodCallbackPolicy methodCallbackPolicyForSelector(
   return readEngineMethodCallbackPolicyValue(runtime, policyValue);
 }
 
+NativeApiMethodCallbackPolicy nativeAccessorCallbackPolicy(
+    NativeApiMethodCallbackPolicy policy) {
+  policy.skipCallbackIfAssociatedObjectTruthy.push_back(
+      "__nativeApiAccessorCallbackState");
+  return policy;
+}
+
 Class dispatchSuperclassForEngineDerivedReceiver(id receiver,
                                                 Class defaultSuperclass) {
   if (receiver == nil) {
@@ -759,9 +766,9 @@ throw JSError(runtime,
           propertyMember->selectorName, propertyMember->signatureOffset,
           (propertyMember->flags & metagen::mdMemberReturnOwned) != 0,
           getter.asObject(runtime).asFunction(runtime),
-          methodCallbackPolicyForSelector(runtime, methodPoliciesPtr,
-                                          propertyName,
-                                          propertyMember->selectorName));
+          nativeAccessorCallbackPolicy(methodCallbackPolicyForSelector(
+              runtime, methodPoliciesPtr, propertyName,
+              propertyMember->selectorName)));
     } else if (propertyMember == nullptr && getter.isObject() &&
                getter.asObject(runtime).isFunction(runtime)) {
       auto overrides = methodOverridesForName(members, propertyName);
@@ -774,9 +781,9 @@ throw JSError(runtime,
             member.signatureOffset,
             (member.flags & metagen::mdMemberReturnOwned) != 0,
             getter.asObject(runtime).asFunction(runtime),
-            methodCallbackPolicyForSelector(runtime, methodPoliciesPtr,
-                                            propertyName,
-                                            member.selectorName));
+            nativeAccessorCallbackPolicy(methodCallbackPolicyForSelector(
+                runtime, methodPoliciesPtr, propertyName,
+                member.selectorName)));
       }
     }
 
@@ -788,9 +795,10 @@ throw JSError(runtime,
                            propertyMember->setterSelectorName,
                            propertyMember->setterSignatureOffset, false,
                            setter.asObject(runtime).asFunction(runtime),
-                           methodCallbackPolicyForSelector(
-                               runtime, methodPoliciesPtr, propertyName,
-                               propertyMember->setterSelectorName));
+                           nativeAccessorCallbackPolicy(
+                               methodCallbackPolicyForSelector(
+                                   runtime, methodPoliciesPtr, propertyName,
+                                   propertyMember->setterSelectorName)));
     }
   }
 
diff --git a/NativeScript/ffi/shared/bridge/HostObject.mm b/NativeScript/ffi/shared/bridge/HostObject.mm
@@ -346,6 +346,36 @@ throw JSError(runtime,
             return Value::undefined();
           });
     }
+    if (property == "__setObjectAccessorCallbackState") {
+      return Function::createFromHostFunction(
+          runtime, PropNameID::forAscii(
+                       runtime, "__setObjectAccessorCallbackState"),
+          2,
+          [](Runtime& runtime, const Value&, const Value* args,
+             size_t count) -> Value {
+            if (count < 1) {
+              return Value::undefined();
+            }
+            id object = NativeApiObjectHostObject::nativeObjectFromValue(
+                runtime, args[0]);
+            if (object == nil) {
+              return Value::undefined();
+            }
+            bool active = count >= 2 && args[1].isBool() && args[1].getBool();
+            SEL key = sel_registerName("__nativeApiAccessorCallbackState");
+            NSNumber* current = (NSNumber*)objc_getAssociatedObject(object, key);
+            NSInteger depth = current != nil ? current.integerValue : 0;
+            if (active) {
+              depth += 1;
+            } else if (depth > 0) {
+              depth -= 1;
+            }
+            objc_setAssociatedObject(
+                object, key, depth > 0 ? @(depth) : nil,
+                OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+            return Value::undefined();
+          });
+    }
     if (property == "CC_SHA256") {
       auto bridge = bridge_;
       return Function::createFromHostFunction(
diff --git a/NativeScript/ffi/shared/bridge/Install.mm b/NativeScript/ffi/shared/bridge/Install.mm
@@ -237,11 +237,51 @@ function findPrototypeDescriptor(className, property) {
     return undefined;
   }
 
+  function setObjectAccessorCallbackState(instance, active) {
+    try {
+      if (typeof api.__setObjectAccessorCallbackState === 'function') {
+        api.__setObjectAccessorCallbackState(instance, !!active);
+      }
+    } catch (_) {
+    }
+  }
+
+  function nativeExtensionAccessorWithCallbackState(fn) {
+    if (typeof fn !== 'function') {
+      return fn;
+    }
+    return function() {
+      setObjectAccessorCallbackState(this, true);
+      try {
+        var args = Array.prototype.slice.call(arguments);
+        return fn.apply(this, args);
+      } finally {
+        setObjectAccessorCallbackState(this, false);
+      }
+    };
+  }
+
   function nativeExtensionMethodsWithIndexedCollectionAliases(methods) {
     if (methods == null || typeof methods !== 'object') {
       return methods;
     }
 
+    var descriptors = Object.getOwnPropertyDescriptors(methods);
+    var descriptorKeys =
+      typeof Reflect === 'object' && typeof Reflect.ownKeys === 'function'
+        ? Reflect.ownKeys(descriptors)
+        : Object.keys(descriptors);
+    var needsAccessorCallbackState = false;
+    for (var descriptorIndex = 0; descriptorIndex < descriptorKeys.length; descriptorIndex++) {
+      var descriptor = descriptors[descriptorKeys[descriptorIndex]];
+      if (descriptor &&
+          (typeof descriptor.get === 'function' ||
+           typeof descriptor.set === 'function')) {
+        needsAccessorCallbackState = true;
+        break;
+      }
+    }
+
     var hasObjectAtIndex =
       Object.prototype.hasOwnProperty.call(methods, 'objectAtIndex');
     var hasCount =
@@ -261,12 +301,31 @@ function nativeExtensionMethodsWithIndexedCollectionAliases(methods) {
 
     if (!needsObjectAtIndexedSubscript &&
         !needsSetObjectAtIndexedSubscript &&
-        !needsIndexedCollectionIterator) {
+        !needsIndexedCollectionIterator &&
+        !needsAccessorCallbackState) {
       return methods;
     }
 
+    if (needsAccessorCallbackState) {
+      for (var accessorIndex = 0; accessorIndex < descriptorKeys.length; accessorIndex++) {
+        var accessorKey = descriptorKeys[accessorIndex];
+        var accessorDescriptor = descriptors[accessorKey];
+        if (!accessorDescriptor) {
+          continue;
+        }
+        if (typeof accessorDescriptor.get === 'function') {
+          accessorDescriptor.get =
+            nativeExtensionAccessorWithCallbackState(accessorDescriptor.get);
+        }
+        if (typeof accessorDescriptor.set === 'function') {
+          accessorDescriptor.set =
+            nativeExtensionAccessorWithCallbackState(accessorDescriptor.set);
+        }
+      }
+    }
+
     var prepared = Object.create(Object.getPrototypeOf(methods));
-    Object.defineProperties(prepared, Object.getOwnPropertyDescriptors(methods));
+    Object.defineProperties(prepared, descriptors);
 
     if (needsObjectAtIndexedSubscript) {
       Object.defineProperty(prepared, 'objectAtIndexedSubscript', {
diff --git a/PROGRESS.md b/PROGRESS.md
@@ -17,6 +17,47 @@ TypeScript/UI worklets.
 
 ## Latest Update - 2026-06-30
 
+### 2026-06-30 04:33 EDT - JS prototype accessors suppress native accessor re-entry
+
+- Scope check:
+  - Still generic NativeScript runtime behavior required by the RN-module
+    branch. No RNS-specific native implementation and no direct-engine backend
+    refactor work.
+  - Simulator-only rule remains active; no physical devices were used.
+- CI finding:
+  - GitHub Actions run `28429299830` on `dc038a4f` compiled and linked the V8
+    native-interceptor patch, kept macOS tests green, and still failed only in
+    iOS:
+    `ApiTests.js SpecialCaseProperty_When_CustomSelector_ImplementedInJS`.
+  - `ApiTests.js NSMutableArrayMethods` still passed.
+  - The failure stayed at `ApiTests.js:347` with hundreds of `getter` logs,
+    so the values were still correct but Objective-C/native accessor re-entry
+    was repeatedly invoking the JS getter while the JS-owned accessor path was
+    active.
+- Change:
+  - JS extension accessor descriptors are now wrapped during `.extend(...)`
+    and TypeScript `NativeClass` materialization so getter/setter bodies run
+    with a depth-counted native associated-object state.
+  - The bridge exposes `__setObjectAccessorCallbackState` to maintain that
+    state on the native object.
+  - Class-builder generated Objective-C callbacks for JS property accessor
+    overrides now compose a generic callback policy that skips native callback
+    re-entry while the JS accessor is executing.
+  - Skipped native callbacks now zero their return storage before applying an
+    optional policy return value, matching the construction-skip behavior.
+- Verification:
+  - Focused source guard passed:
+    `node packages/react-native/test/runtime-instance-selector-base-dispatch.test.js`.
+  - Runtime RN JS tests passed:
+    `for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
+  - `git diff --check` passed.
+  - `npm run check:ffi-boundaries` passed.
+  - Existing generated macOS project compile/link passed with
+    `xcodebuild -project dist/intermediates/macos/NativeScript.xcodeproj ...`.
+- Still next:
+  - Commit/push this accessor re-entry guard and watch fresh PR CI. If iOS goes
+    green, resume the RNS simulator parity sweep from this handoff.
+
 ### 2026-06-30 03:57 EDT - V8 native instance interceptors honor JS prototype accessors
 
 - Scope check:
diff --git a/packages/react-native/test/runtime-instance-selector-base-dispatch.test.js b/packages/react-native/test/runtime-instance-selector-base-dispatch.test.js
@@ -145,6 +145,57 @@ for (const relativePath of [
   );
 }
 
+{
+  const installSource = fs.readFileSync(
+    path.join(repoRoot, "NativeScript/ffi/shared/bridge/Install.mm"),
+    "utf8",
+  );
+  const hostObjectSource = fs.readFileSync(
+    path.join(repoRoot, "NativeScript/ffi/shared/bridge/HostObject.mm"),
+    "utf8",
+  );
+  const classBuilderSource = fs.readFileSync(
+    path.join(repoRoot, "NativeScript/ffi/shared/bridge/ClassBuilder.mm"),
+    "utf8",
+  );
+  const callbacksSource = fs.readFileSync(
+    path.join(repoRoot, "NativeScript/ffi/shared/bridge/Callbacks.mm"),
+    "utf8",
+  );
+
+  assert(
+    installSource.includes("function setObjectAccessorCallbackState(instance, active)") &&
+      installSource.includes("__setObjectAccessorCallbackState(instance, !!active)") &&
+      installSource.includes("function nativeExtensionAccessorWithCallbackState(fn)") &&
+      installSource.includes("Object.getOwnPropertyDescriptors(methods)") &&
+      installSource.includes("Reflect.ownKeys(descriptors)") &&
+      installSource.includes("fn.apply(this, args)") &&
+      installSource.includes("setObjectAccessorCallbackState(this, false)"),
+    "JS extension accessors should run with native callback re-entry suppression state",
+  );
+  assert(
+    hostObjectSource.includes("__setObjectAccessorCallbackState") &&
+      hostObjectSource.includes('sel_registerName("__nativeApiAccessorCallbackState")') &&
+      hostObjectSource.includes("depth += 1") &&
+      hostObjectSource.includes("depth -= 1") &&
+      hostObjectSource.includes("depth > 0 ? @(depth) : nil"),
+    "bridge API should expose a depth-counted JS accessor callback state on native objects",
+  );
+  assert(
+    classBuilderSource.includes("NativeApiMethodCallbackPolicy nativeAccessorCallbackPolicy") &&
+      classBuilderSource.includes('"__nativeApiAccessorCallbackState"') &&
+      classBuilderSource.includes("nativeAccessorCallbackPolicy(methodCallbackPolicyForSelector") &&
+      classBuilderSource.match(/nativeAccessorCallbackPolicy\(methodCallbackPolicyForSelector/g)?.length >= 2 &&
+      classBuilderSource.includes("nativeAccessorCallbackPolicy(\n                               methodCallbackPolicyForSelector"),
+    "property accessor overrides should skip native callback re-entry while JS accessors are executing",
+  );
+  assert(
+    callbacksSource.includes("zeroReturnValue(ret);\n        applyMethodPolicyAssignments(args);") &&
+      callbacksSource.includes("zeroReturnValue(ret);\n    applyMethodPolicyAssignments(args);"),
+    "skipped native method callbacks should zero their native return storage before applying policy return values",
+  );
+}
+
 for (const relativePath of [
   "NativeScript/ffi/shared/bridge/ClassBuilder.mm",
   "packages/react-native/native-api/ffi/shared/bridge/ClassBuilder.mm",
