Skip to content

Commit 99889cb

Browse files
committed
fix: stabilize object expando runtime keys
1 parent 7eae675 commit 99889cb

4 files changed

Lines changed: 102 additions & 9 deletions

File tree

HANDOFF.md

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@ reliability, modal behavior, stack/tab behavior, and simulator polish.
2929
`codex/rn-module-fabric-turbomodule-worklets`
3030
- Created from `refactor` at
3131
`f3d0b3f4ac6f6ff5753d321e7bb7ecc7e78f3443`.
32-
- Branch split audit from 2026-06-28 23:36 EDT:
33-
`HEAD`, `refactor`, and their merge-base all resolve to `f3d0b3f4`.
34-
`git diff refactor...HEAD` is empty, so no RN-module commits have landed
35-
on top of `refactor` yet. The RN work is currently dirty working-tree
36-
state on this RN branch.
32+
- Branch split audit from 2026-06-28 23:36 EDT confirmed the RN work was
33+
separated from `refactor`; subsequent RN/runtime primitive commits now live
34+
on `codex/rn-module-fabric-turbomodule-worklets` and are tracked in draft
35+
PR #46.
3736
- Keep this branch for RN module / Fabric / TurboModule / worklets work.
3837
Preserve the original `refactor` branch for the Node-API runtime/direct
3938
engine backend PR.
@@ -111,6 +110,44 @@ During this thread `agent-device` was version `0.18.0`; the npm package is
111110

112111
## Current Verified State
113112

113+
Update from 2026-06-29 16:55 EDT:
114+
115+
- Simulator-only rule remains active. Do not use physical iPhone/iPad devices.
116+
- Draft PR #46 branch is cleanly on
117+
`codex/rn-module-fabric-turbomodule-worklets`; the previous writable-member
118+
correction was committed/pushed as `7eae675d`.
119+
- GitHub Actions run `28399361627` on `7eae675d` still failed only in iOS
120+
simulator `ApiTests.js Appearance`:
121+
`UILabel.appearance().textColor` read back `undefined` after assignment.
122+
macOS stayed green.
123+
- Root cause refinement:
124+
- The failed read can happen even if the UIAppearance setter path succeeds.
125+
- Root runtime object expandos were keyed by `&runtime`, but V8 host-object
126+
callbacks construct fresh stack `Runtime` wrappers for get/set calls. That
127+
makes a value cached during assignment invisible to the later read under a
128+
different wrapper address.
129+
- Current runtime fix:
130+
- Root `NativeApiBridge` object expandos now key per-runtime values by stable
131+
backend runtime state for V8/JSC/QuickJS via `runtime.state().get()`.
132+
- Hermes/JSI keeps using the real JSI runtime reference fallback.
133+
- Source coverage now guards against reintroducing the per-callback stack
134+
wrapper key in root object expandos.
135+
- Local verification after this correction:
136+
- `node packages/react-native/test/runtime-objc-property-setter.test.js`
137+
passed.
138+
- `git diff --check` passed.
139+
- `npm run check:ffi-boundaries` passed.
140+
- Runtime RN JS tests passed:
141+
`for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
142+
- `npm run build:macos-cli` passed and compiled/linked the edited V8 bridge.
143+
- Narrow local iOS simulator repro could not launch because the local
144+
metadata-generator x86_64 build still fails to link against the arm64-only
145+
Xcode `libclang.dylib`; keep using GitHub Actions for the authoritative iOS
146+
simulator result until that local toolchain mismatch is fixed.
147+
- Not done:
148+
- Commit/push the stable runtime-expando key fix and watch fresh PR CI.
149+
- If CI turns green, resume the dedicated simulator-only RNS parity sweep.
150+
114151
Update from 2026-06-29 16:05 EDT:
115152

116153
- Simulator-only rule remains active. Do not use physical iPhone/iPad devices.

NativeScript/ffi/shared/bridge/ObjCBridge.mm

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,16 @@ inline uintptr_t normalizeRuntimePointer(uintptr_t pointer) {
529529
#endif
530530
}
531531

532+
uintptr_t runtimeObjectExpandoKey(Runtime& runtime) {
533+
#if defined(TARGET_ENGINE_V8) || defined(TARGET_ENGINE_JSC) || \
534+
defined(TARGET_ENGINE_QUICKJS)
535+
return normalizeRuntimePointer(
536+
reinterpret_cast<uintptr_t>(runtime.state().get()));
537+
#else
538+
return normalizeRuntimePointer(reinterpret_cast<uintptr_t>(&runtime));
539+
#endif
540+
}
541+
532542
class NativeApiBridge {
533543
struct NativeApiRoundTripValue {
534544
std::shared_ptr<Value> value;
@@ -958,8 +968,7 @@ void setObjectExpando(Runtime& runtime, const void* native,
958968
}
959969
const uintptr_t key =
960970
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
961-
const uintptr_t runtimeKey =
962-
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(&runtime));
971+
const uintptr_t runtimeKey = runtimeObjectExpandoKey(runtime);
963972
{
964973
std::lock_guard<std::mutex> lock(objectExpandosMutex_);
965974
objectExpandos_[key][property][runtimeKey] =
@@ -987,8 +996,7 @@ Value findObjectExpando(Runtime& runtime, const void* native,
987996

988997
const uintptr_t key =
989998
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(native));
990-
const uintptr_t runtimeKey =
991-
normalizeRuntimePointer(reinterpret_cast<uintptr_t>(&runtime));
999+
const uintptr_t runtimeKey = runtimeObjectExpandoKey(runtime);
9921000
const uint64_t generation =
9931001
objectExpandosGeneration_.load(std::memory_order_acquire);
9941002
for (auto& entry : cache) {

PROGRESS.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,43 @@ TypeScript/UI worklets.
1717

1818
## Latest Update - 2026-06-29
1919

20+
### 2026-06-29 16:55 EDT - stable runtime keys for object expandos
21+
22+
- Goal:
23+
- Keep PR #46 on the generic runtime primitive path and remain
24+
simulator-only. No physical devices were used.
25+
- CI finding:
26+
- GitHub Actions run `28399361627` on `7eae675d` kept macOS green but still
27+
failed iOS simulator `ApiTests.js Appearance`.
28+
- The failure was unchanged: `UILabel.appearance().textColor` read back
29+
`undefined` after assignment.
30+
- Refinement: the setter/cache path could succeed and still miss on read
31+
because root object expandos were keyed by `&runtime`. V8 host-object
32+
callbacks create fresh stack `Runtime` wrappers, so assignment and read can
33+
use different wrapper addresses for the same backend runtime.
34+
- Changes:
35+
- Added a stable root `runtimeObjectExpandoKey` helper.
36+
- V8/JSC/QuickJS object expandos now key per-runtime values by
37+
`runtime.state().get()` instead of the callback wrapper address.
38+
- Hermes/JSI keeps the real runtime reference fallback.
39+
- Source coverage now guards against reintroducing the stack-wrapper key for
40+
root object expandos.
41+
- Verification:
42+
- `node packages/react-native/test/runtime-objc-property-setter.test.js`
43+
passed.
44+
- `git diff --check` passed.
45+
- `npm run check:ffi-boundaries` passed.
46+
- Runtime RN JS tests passed:
47+
`for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
48+
- `npm run build:macos-cli` passed and compiled/linked the edited V8 bridge.
49+
- Narrow local iOS simulator repro is still blocked before launch by the
50+
local x86_64 metadata-generator link mismatch against Xcode's arm64-only
51+
`libclang.dylib`.
52+
- Still next:
53+
- Commit/push this stable runtime-expando key correction and watch fresh PR
54+
CI for the authoritative iOS simulator result.
55+
- If CI turns green, resume the dedicated simulator-only RNS parity sweep.
56+
2057
### 2026-06-29 16:05 EDT - UIAppearance writable member selection
2158

2259
- Goal:

packages/react-native/test/runtime-objc-property-setter.test.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ const runtimeHostObjects = fs.readFileSync(
4040
path.join(repoRoot, "NativeScript/ffi/shared/bridge/HostObjects.mm"),
4141
"utf8",
4242
);
43+
const runtimeObjCBridge = fs.readFileSync(
44+
path.join(repoRoot, "NativeScript/ffi/shared/bridge/ObjCBridge.mm"),
45+
"utf8",
46+
);
4347
const appearanceAccessorStart = runtimeHostObjects.indexOf(
4448
"Function makeAppearanceProxyPropertySetter(",
4549
);
@@ -104,5 +108,12 @@ assert(
104108
!runtimeHostObjects.includes("SetNativeApiObjectPrototype(runtime, resultObject"),
105109
"runtime UIAppearance proxies should not replace their JS prototype with the target class prototype",
106110
);
111+
assert(
112+
runtimeObjCBridge.includes("uintptr_t runtimeObjectExpandoKey(Runtime& runtime)") &&
113+
runtimeObjCBridge.includes("runtime.state().get()") &&
114+
runtimeObjCBridge.includes("const uintptr_t runtimeKey = runtimeObjectExpandoKey(runtime);") &&
115+
!runtimeObjCBridge.includes("const uintptr_t runtimeKey =\n normalizeRuntimePointer(reinterpret_cast<uintptr_t>(&runtime));"),
116+
"runtime object expandos should use stable backend runtime identity instead of per-callback stack wrapper addresses",
117+
);
107118

108119
console.log("runtime Objective-C property setter tests passed");

0 commit comments

Comments
 (0)