Skip to content

Commit 85cbf91

Browse files
Gasan Aknievmeta-codesync[bot]
authored andcommitted
Fix residual EXC_BREAKPOINT crashes in FBObjectiveCObject.allRetainedObjects
Summary: After D103430527 reduced the EXC_BREAKPOINT crashes by ~95% in alpha/beta builds on Instagram and Threads/Barcelona, two residual crash modes still appear in alpha/beta at low rates (no production exposure yet): 1. swift_unknownObjectRetain crash inside the objc bridge of SwiftIntrospector.getPropertyValue. The Mirror API can return stale pointers for unowned references whose target was deallocated between the property assignment and the RCD traversal. The previous fix filtered out value types, but stale class instance pointers still passed through. 2. _class_lookUpIvar / object_getIvar crash when the parent object is released by another thread mid-traversal. The detector previously used objectPtr (a raw void*) which doesn't keep the object alive across the method. Fixes: - SwiftIntrospector.swift: Add isLikelyLiveObject() validation using malloc_size to filter out dangling pointers. Tagged pointers (high bit on arm64, low bit on x86_64) are handled separately since they aren't heap-allocated. Class instances and Foundation-bridgeable values are now liveness-checked before bridging to ObjC id. - FBObjectiveCObject.m: Pin the object alive for the entire allRetainedObjects method via a strong local. For ObjC objects, reading self.object via the weak property gives us a brief strong reference. For pure Swift objects, the FBSwiftStrongRef wrapper held by the candidates array continues to keep the object alive. Both crash signatures observed in alpha/beta builds: - Instagram T268028126 (instagram_ios_crashes:69bd7a32c0a579e93bee27efe143c89a) - Threads T268462327 (barcelona_ios_crashes:0132b4a00a546d59009ed4420a32303b) Sample stack 1 (swift_unknownObjectRetain): swift_unknownObjectRetain objc static SwiftIntrospector.getPropertyValue(object:name:) -[FBObjectiveCObject allRetainedObjects] -[FBNodeEnumerator nextObject] -[FBRetainCycleDetector _findRetainCyclesInObject:stackDepth:] Sample stack 2 (object_getIvar): _class_lookUpIvar object_getIvar -[FBObjectiveCObject allRetainedObjects] -[FBNodeEnumerator nextObject] -[FBRetainCycleDetector _findRetainCyclesInObject:stackDepth:] This is a preventive fix to ensure the residual crashes do not leak into production. Reviewed By: thegreatwallfb Differential Revision: D104141469 fbshipit-source-id: 66fd98e5e360a7fca10fe8e9f89fba7834f9b510
1 parent 59857ae commit 85cbf91

2 files changed

Lines changed: 43 additions & 9 deletions

File tree

FBRetainCycleDetector/Graph/FBObjectiveCObject.m

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,22 @@ @implementation FBObjectiveCObject
1919

2020
- (NSSet *)allRetainedObjects
2121
{
22-
void *ptr = [self objectPtr];
23-
if (!ptr) {
24-
return nil;
22+
// Pin the object alive for the entire method. For ObjC objects, reading
23+
// self.object via the weak property gives us a brief strong reference
24+
// that lives for the scope of strongObj. For pure Swift objects, the
25+
// FBSwiftStrongRef wrapper held by the candidates array keeps the
26+
// object alive across this method's invocation. Without pinning,
27+
// another thread can release the object mid-traversal, causing
28+
// crashes in object_getIvar / class_lookUpIvar.
29+
__strong id strongObj = self.object;
30+
void *ptr;
31+
if (strongObj) {
32+
ptr = (__bridge void *)strongObj;
33+
} else {
34+
ptr = [self objectPtr];
35+
if (!ptr) {
36+
return nil;
37+
}
2538
}
2639
__unsafe_unretained id obj = (__bridge id)ptr;
2740

FBRetainCycleDetector/Layout/Classes/SwiftIntrospector.swift

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* LICENSE file in the root directory of this source tree.
88
*/
99

10+
import Darwin
1011
import Foundation
1112

1213
@objc public class SwiftIntrospector: NSObject {
@@ -43,9 +44,14 @@ import Foundation
4344
}
4445

4546
private class func asRetainableObject(_ value: Any) -> AnyObject? {
46-
// Class instances are always safe to bridge to id.
47+
// Class instances are safe to bridge to id, but we must verify the
48+
// pointer is still live. Mirror can return stale pointers for
49+
// unowned references whose target was deallocated. Bridging a
50+
// dangling pointer back to ObjC id triggers swift_unknownObjectRetain
51+
// crashes during the @objc return-value retain.
4752
if Swift.type(of: value) is AnyClass {
48-
return value as AnyObject
53+
let obj = value as AnyObject
54+
return isLikelyLiveObject(obj) ? obj : nil
4955
}
5056
// Optional wrapping a class instance — unwrap and check.
5157
let valueMirror = Mirror(reflecting: value)
@@ -57,15 +63,30 @@ import Foundation
5763
}
5864
// Foundation-bridgeable value types (String→NSString, Array→NSArray, etc.)
5965
// are safe. Non-bridgeable Swift structs/enums will crash on `as AnyObject`.
60-
// Check _ObjectiveCBridgeable conformance via the bridged result's identity:
61-
// if bridging produces an object whose type is a class, it's safe.
62-
// We use a two-step check to avoid crashing on non-bridgeable types.
6366
if value is NSObject {
64-
return value as AnyObject
67+
let obj = value as AnyObject
68+
return isLikelyLiveObject(obj) ? obj : nil
6569
}
6670
return nil
6771
}
6872

73+
/// Verify the object's pointer is still in a live heap allocation.
74+
/// Tagged pointers (small NSNumber, short NSString) are not heap-allocated
75+
/// and pass through unchanged.
76+
private class func isLikelyLiveObject(_ obj: AnyObject) -> Bool {
77+
let raw = Unmanaged.passUnretained(obj).toOpaque()
78+
let intPtr = Int(bitPattern: raw)
79+
// Tagged pointers: high bit set on arm64, low bit on x86_64.
80+
// These are not heap allocations — accept them.
81+
#if arch(arm64) || arch(arm64_32)
82+
if intPtr < 0 { return true }
83+
#else
84+
if (intPtr & 1) != 0 { return true }
85+
#endif
86+
// Heap-allocated pointers must have a non-zero malloc size.
87+
return malloc_size(raw) > 0
88+
}
89+
6990
private class func getChild(mirror: Mirror, name: String) -> (Any?, Bool) {
7091
guard let array = AnyBidirectionalCollection(mirror.children) else {
7192
return (nil, false)

0 commit comments

Comments
 (0)