Skip to content

Commit 3b7a492

Browse files
committed
fix: guard JS callbacks during subclass init
1 parent 59a756f commit 3b7a492

5 files changed

Lines changed: 144 additions & 7 deletions

File tree

NativeScript/ffi/shared/bridge/Callbacks.mm

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,10 @@ bool shouldSkipMethodCallback(void* args[], void* ret) {
14501450
return false;
14511451
}
14521452

1453+
if (shouldSkipConstructingMethodCallback(args, ret)) {
1454+
return true;
1455+
}
1456+
14531457
id receiver = objectForMethodPolicyTarget(
14541458
args, NativeApiMethodCallbackPolicy::Target{});
14551459
for (const auto& key :
@@ -1470,6 +1474,23 @@ bool shouldSkipMethodCallback(void* args[], void* ret) {
14701474
return true;
14711475
}
14721476

1477+
bool shouldSkipConstructingMethodCallback(void* args[], void* ret) {
1478+
if (!bindThis_ || args == nullptr || signature_ == nullptr ||
1479+
signature_->selectorName.rfind("init", 0) == 0) {
1480+
return false;
1481+
}
1482+
1483+
id receiver = *static_cast<id*>(args[0]);
1484+
if (receiver == nil ||
1485+
objc_getAssociatedObject(
1486+
receiver, sel_registerName("__nativeApiConstructionState")) == nil) {
1487+
return false;
1488+
}
1489+
1490+
zeroReturnValue(ret);
1491+
return true;
1492+
}
1493+
14731494
void invokeMethodSuper(void* ret, void* args[]) const {
14741495
if (!bindThis_ || args == nullptr || signature_ == nullptr ||
14751496
methodBaseClass_ == Nil) {

NativeScript/ffi/shared/bridge/HostObject.mm

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,28 @@ throw JSError(runtime,
324324
return Value::undefined();
325325
});
326326
}
327+
if (property == "__setObjectConstructionState") {
328+
return Function::createFromHostFunction(
329+
runtime, PropNameID::forAscii(runtime, "__setObjectConstructionState"),
330+
2,
331+
[](Runtime& runtime, const Value&, const Value* args,
332+
size_t count) -> Value {
333+
if (count < 1) {
334+
return Value::undefined();
335+
}
336+
id object = NativeApiObjectHostObject::nativeObjectFromValue(
337+
runtime, args[0]);
338+
if (object == nil) {
339+
return Value::undefined();
340+
}
341+
bool constructing =
342+
count >= 2 && args[1].isBool() && args[1].getBool();
343+
objc_setAssociatedObject(
344+
object, sel_registerName("__nativeApiConstructionState"),
345+
constructing ? @YES : nil, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
346+
return Value::undefined();
347+
});
348+
}
327349
if (property == "CC_SHA256") {
328350
auto bridge = bridge_;
329351
return Function::createFromHostFunction(
@@ -533,6 +555,7 @@ throw JSError(runtime,
533555
addPropertyName(runtime, names, "__makeSelectorGroupFunction");
534556
addPropertyName(runtime, names, "__rememberClassWrapper");
535557
addPropertyName(runtime, names, "__rememberObjectClassWrapper");
558+
addPropertyName(runtime, names, "__setObjectConstructionState");
536559
addPropertyName(runtime, names, "getFunction");
537560
addPropertyName(runtime, names, "getConstant");
538561
addPropertyName(runtime, names, "getEnum");

NativeScript/ffi/shared/bridge/Install.mm

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,16 @@ function shouldUseAllocInitConstructor(constructable, wrapper) {
635635
}
636636
}
637637
638-
function constructNativeInstance(nativeClass, args, rememberInstance) {
638+
function setObjectConstructionState(instance, constructing) {
639+
try {
640+
if (api && typeof api.__setObjectConstructionState === 'function') {
641+
api.__setObjectConstructionState(instance, !!constructing);
642+
}
643+
} catch (_) {
644+
}
645+
}
646+
647+
function constructNativeInstance(nativeClass, args, rememberInstance, markConstructing) {
639648
if (args.length === 1 &&
640649
args[0] &&
641650
typeof args[0] === 'object' &&
@@ -674,13 +683,16 @@ function constructNativeInstance(nativeClass, args, rememberInstance) {
674683
if (typeof rememberInstance === 'function') {
675684
instance = rememberInstance(instance);
676685
}
677-
if (initializer.selectorName === 'init') {
678-
if (typeof instance.init !== 'function') {
679-
throw new Error('No initializer found that matches constructor invocation.');
680-
}
681-
return instance.init();
686+
if (markConstructing) {
687+
setObjectConstructionState(instance, true);
682688
}
683689
try {
690+
if (initializer.selectorName === 'init') {
691+
if (typeof instance.init !== 'function') {
692+
throw new Error('No initializer found that matches constructor invocation.');
693+
}
694+
return instance.init();
695+
}
684696
if (initializer.name && typeof instance[initializer.name] === 'function') {
685697
return instance[initializer.name](...actualArgs);
686698
}
@@ -694,6 +706,10 @@ function constructNativeInstance(nativeClass, args, rememberInstance) {
694706
throw new Error('No initializer found that matches constructor invocation.');
695707
}
696708
throw error;
709+
} finally {
710+
if (markConstructing) {
711+
setObjectConstructionState(instance, false);
712+
}
697713
}
698714
}
699715
@@ -795,7 +811,12 @@ function wrapNativeClass(nativeClass) {
795811
}
796812
if (args.length > 0 ||
797813
shouldUseAllocInitConstructor(constructable, wrapper)) {
798-
return rememberInstanceClass(constructNativeInstance(nativeClass, args, rememberInstanceClass));
814+
return rememberInstanceClass(constructNativeInstance(
815+
nativeClass,
816+
args,
817+
rememberInstanceClass,
818+
shouldUseAllocInitConstructor(constructable, wrapper)
819+
));
799820
}
800821
if (typeof nativeClass.new !== 'function') {
801822
throw new Error('Native class cannot be initialized');

PROGRESS.md

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

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

20+
### 2026-06-30 01:13 EDT - guard JS callbacks during subclass construction
21+
22+
- Scope check:
23+
- This remains runtime stabilization on the RN-module branch split from
24+
`refactor`. The change is a generic NativeScript JS-subclass construction
25+
policy, not an RNS workaround and not direct-engine backend refactor work.
26+
- Simulator-only rule remains active; no physical devices were used.
27+
- CI finding:
28+
- GitHub Actions run `28420460035` on `59a756f4` kept setup, dependency
29+
install, FFI boundary check, V8 download, libffi build, metadata
30+
generation, NativeScript build, CLI build, and macOS tests green.
31+
- iOS simulator still timed out at `ApiTests.js NSMutableArrayMethods`.
32+
- The remaining failure is now narrowed to pre/post-init JS-backed subclass
33+
callback behavior on iOS, not macOS wrapper identity.
34+
- Change:
35+
- Added a host API that marks native objects as being inside JS-subclass
36+
alloc/init construction.
37+
- The install-time alloc/init constructor path marks only JS-extended
38+
constructors that opt into remembered alloc/init construction, then clears
39+
the marker in a `finally` block.
40+
- Objective-C method callbacks now zero-return non-`init` JS overrides while
41+
that receiver is still constructing, avoiding Foundation probes running JS
42+
subclass methods before JS state exists.
43+
- Source coverage guards the tracked runtime bridge and the RN mirror.
44+
- Verification:
45+
- Focused construction guard test passed:
46+
`node packages/react-native/test/runtime-instance-selector-base-dispatch.test.js`.
47+
- Runtime RN JS tests passed:
48+
`for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
49+
- `git diff --check` passed.
50+
- `npm run check:ffi-boundaries` passed.
51+
- `npm run build:macos-cli` passed.
52+
- Still next:
53+
- Commit/push this construction-state callback guard and watch fresh PR CI for
54+
the iOS simulator `ApiTests.js NSMutableArrayMethods` result.
55+
- If iOS still hangs, add a tightly scoped diagnostic to distinguish
56+
construction-time probing from fast-enumeration wrapper/state loss, then
57+
remove it before landing.
58+
2059
### 2026-06-30 00:32 EDT - preserved init receivers use native-object round-trip keys
2160

2261
- Scope check:

packages/react-native/test/runtime-instance-selector-base-dispatch.test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,47 @@ for (const relativePath of [
3030
source.match(/Object\.defineProperty\([^,]+, '__nativeApiUseAllocInitConstructor'/g) || [];
3131
assert(
3232
source.includes("function shouldUseAllocInitConstructor(constructable, wrapper)") &&
33+
source.includes("function setObjectConstructionState(instance, constructing)") &&
34+
source.includes("api.__setObjectConstructionState(instance, !!constructing)") &&
3335
source.includes("target.__nativeApiUseAllocInitConstructor") &&
3436
source.includes("args.length > 0 ||") &&
3537
source.includes("shouldUseAllocInitConstructor(constructable, wrapper)") &&
38+
source.includes("setObjectConstructionState(instance, true)") &&
39+
source.includes("setObjectConstructionState(instance, false)") &&
3640
allocInitFlagDefinitions.length >= 2,
3741
`${relativePath}: JS-extended native classes should use alloc/init construction so receivers are remembered before init dispatch`,
3842
);
3943
}
4044

45+
for (const relativePath of [
46+
"NativeScript/ffi/shared/bridge/HostObject.mm",
47+
"packages/react-native/native-api/ffi/shared/bridge/HostObject.mm",
48+
]) {
49+
const source = fs.readFileSync(path.join(repoRoot, relativePath), "utf8");
50+
51+
assert(
52+
source.includes("__setObjectConstructionState") &&
53+
source.includes('sel_registerName("__nativeApiConstructionState")') &&
54+
source.includes("constructing ? @YES : nil"),
55+
`${relativePath}: bridge API should mark native objects during JS-subclass construction`,
56+
);
57+
}
58+
59+
for (const relativePath of [
60+
"NativeScript/ffi/shared/bridge/Callbacks.mm",
61+
"packages/react-native/native-api/ffi/shared/bridge/Callbacks.mm",
62+
]) {
63+
const source = fs.readFileSync(path.join(repoRoot, relativePath), "utf8");
64+
65+
assert(
66+
source.includes("shouldSkipConstructingMethodCallback(args, ret)") &&
67+
source.includes('signature_->selectorName.rfind("init", 0) == 0') &&
68+
source.includes('sel_registerName("__nativeApiConstructionState")') &&
69+
source.includes("zeroReturnValue(ret);"),
70+
`${relativePath}: Objective-C callbacks should zero-return non-init JS overrides while the receiver is still constructing`,
71+
);
72+
}
73+
4174
for (const relativePath of [
4275
"NativeScript/ffi/shared/bridge/ClassBuilder.mm",
4376
"packages/react-native/native-api/ffi/shared/bridge/ClassBuilder.mm",

0 commit comments

Comments
 (0)