ref: Use weak references for ANRTracker listeners (#2742)

Replace NSMutableSet with NSHashTable and use weakObjectsHashTable to avoid retaining strong references to ANR tracker listeners.
getsentry · Mar 3, 2023 · 78d5983 · 78d5983
1 parent dbc67d2
commit 78d5983
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 2 deletions.
diff --git a/Sources/Sentry/SentryANRTracker.m b/Sources/Sentry/SentryANRTracker.m
@@ -20,7 +20,7 @@ typedef NS_ENUM(NSInteger, SentryANRTrackerState) {
 @property (nonatomic, strong) SentryCrashWrapper *crashWrapper;
 @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper;
 @property (nonatomic, strong) SentryThreadWrapper *threadWrapper;
-@property (nonatomic, strong) NSMutableSet<id<SentryANRTrackerDelegate>> *listeners;
+@property (nonatomic, strong) NSHashTable<id<SentryANRTrackerDelegate>> *listeners;
 @property (nonatomic, assign) NSTimeInterval timeoutInterval;
 
 @end
@@ -42,7 +42,7 @@ - (instancetype)initWithTimeoutInterval:(NSTimeInterval)timeoutInterval
         self.crashWrapper = crashWrapper;
         self.dispatchQueueWrapper = dispatchQueueWrapper;
         self.threadWrapper = threadWrapper;
-        self.listeners = [NSMutableSet set];
+        self.listeners = [NSHashTable weakObjectsHashTable];
         threadLock = [[NSObject alloc] init];
         state = kSentryANRTrackerNotRunning;
     }

diff --git a/Sources/Sentry/SentryANRTrackingIntegration.m b/Sources/Sentry/SentryANRTrackingIntegration.m
@@ -55,6 +55,11 @@ - (void)uninstall
     [self.tracker removeListener:self];
 }
 
+- (void)dealloc
+{
+    [self uninstall];
+}
+
 - (void)anrDetected
 {
     SentryThreadInspector *threadInspector = SentrySDK.currentHub.getClient.threadInspector;

diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackerTests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackerTests.swift
@@ -156,6 +156,29 @@ class SentryANRTrackerTests: XCTestCase, SentryANRTrackerDelegate {
 
     }
 
+    func testNotRemovingDeallocatedListener_DoesNotRetainListener_AndStopsTracking() {
+        anrDetectedExpectation.isInverted = true
+        anrStoppedExpectation.isInverted = true
+
+        // So ARC deallocates SentryANRTrackerTestDelegate
+        let addListenersCount = 10
+        func addListeners() {
+            for _ in 0..<addListenersCount {
+                self.sut.addListener(SentryANRTrackerTestDelegate())
+            }
+        }
+        addListeners()
+
+        sut.addListener(self)
+        sut.removeListener(self)
+
+        let listeners = Dynamic(sut).listeners.asObject as? NSHashTable<NSObject>
+
+        XCTAssertGreaterThan(addListenersCount, listeners?.count ?? addListenersCount)
+
+        wait(for: [anrDetectedExpectation, anrStoppedExpectation], timeout: 0.0)
+    }
+
     func testClearDirectlyAfterStart() {
         anrDetectedExpectation.isInverted = true
 

diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift
@@ -107,6 +107,25 @@ class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase {
 
         assertNoEventCaptured()
     }
+
+    func testDealloc_CallsUninstall() {
+        givenInitializedTracker()
+
+        // // So ARC deallocates the SentryANRTrackingIntegration
+        func initIntegration() {
+            self.crashWrapper.internalIsBeingTraced = false
+            let sut = SentryANRTrackingIntegration()
+            sut.install(with: self.options)
+        }
+
+        initIntegration()
+
+        let tracker = SentryDependencyContainer.sharedInstance().getANRTracker(self.options.appHangTimeoutInterval)
+
+        let listeners = Dynamic(tracker).listeners.asObject as? NSHashTable<NSObject>
+
+        XCTAssertEqual(1, listeners?.count ?? 2)
+    }
 
     private func givenInitializedTracker(isBeingTraced: Bool = false) {
         givenSdkWithHub()