commit 915fb2c2e37db143e3f5e62e59e94a008c0ffdbd
author Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Thu Oct 17 18:13:21 2024 -0700
committer GitHub <noreply@github.com> Fri Oct 18 01:13:21 2024 +0000
tree 76189a72b81da8ffe270501e0280b2f5d5dd86fa
parent 3a6c54500b1cf9ef38b5e304c78399c933903d82

[Darwin] Some MTRDevice mtr_strongify sites need to guard against nil self (#36131)

* [Darwin] Some MTRDevice mtr_strongify sites need to guard against nil self

* Took ksperling-apple's suggestion to use VerifyOrReturn instead for cleaner code

src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
index 68b610c..2342cad 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
@@ -389,6 +389,8 @@
             mtr_weakify(self);
             _systemTimeChangeObserverToken = [[NSNotificationCenter defaultCenter] addObserverForName:NSSystemClockDidChangeNotification object:nil queue:nil usingBlock:^(NSNotification * _Nonnull notification) {
                 mtr_strongify(self);
+                VerifyOrReturn(self);
+
                 std::lock_guard lock(self->_lock);
                 [self _resetStorageBehaviorState];
             }];
@@ -760,6 +762,8 @@
             mtr_weakify(self);
             [self _scheduleSubscriptionPoolWork:^{
                 mtr_strongify(self);
+                VerifyOrReturn(self);
+
                 [self->_deviceController asyncDispatchToMatterQueue:^{
                     std::lock_guard lock(self->_lock);
                     [self _setupSubscriptionWithReason:[NSString stringWithFormat:@"%@ and scheduled subscription is happening", reason]];
@@ -1168,16 +1172,16 @@
     mtr_weakify(self);
     dispatch_block_t workBlockToQueue = ^{
         mtr_strongify(self);
-        if (nil == self) {
-            // This block may be delayed by a specified number of nanoseconds, potentially running after the device is deallocated.
-            // If so, MTRAsyncWorkItem::initWithQueue will assert on a nil queue, which will cause a crash.
-            return;
-        }
+        // This block may be delayed by a specified number of nanoseconds, potentially running after the device is deallocated.
+        // If so, MTRAsyncWorkItem::initWithQueue will assert on a nil queue, which will cause a crash.
+        VerifyOrReturn(self);
 
         // In the case where a resubscription triggering event happened and already established, running the work block should result in a no-op
         MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:self.queue];
         [workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull completion) {
             mtr_strongify(self);
+            VerifyOrReturn(self);
+
             MTR_LOG("%@ - work item is ready to attempt pooled subscription", self);
             os_unfair_lock_lock(&self->_lock);
 #ifdef DEBUG
@@ -1257,6 +1261,8 @@
     mtr_weakify(self);
     auto resubscriptionBlock = ^{
         mtr_strongify(self);
+        VerifyOrReturn(self);
+
         [self->_deviceController asyncDispatchToMatterQueue:^{
             [self _triggerResubscribeWithReason:@"ResubscriptionNeeded timer fired" nodeLikelyReachable:NO];
         } errorHandler:^(NSError * _Nonnull error) {
@@ -1369,6 +1375,8 @@
     mtr_weakify(self);
     auto resubscriptionBlock = ^{
         mtr_strongify(self);
+        VerifyOrReturn(self);
+
         [self->_deviceController asyncDispatchToMatterQueue:^{
             std::lock_guard lock(self->_lock);
             [self _reattemptSubscriptionNowIfNeededWithReason:@"got subscription reset"];
@@ -2386,9 +2394,9 @@
         mtr_weakify(self);
         dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast<int64_t>(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast<int64_t>(NSEC_PER_SEC)), self.queue, ^{
             mtr_strongify(self);
-            if (self != nil) {
-                [self _markDeviceAsUnreachableIfNeverSubscribed];
-            }
+            VerifyOrReturn(self);
+
+            [self _markDeviceAsUnreachableIfNeverSubscribed];
         });
     }
 
@@ -3242,6 +3250,8 @@
         mtr_weakify(self);
         dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (waitTime * NSEC_PER_SEC)), self.queue, ^{
             mtr_strongify(self);
+            VerifyOrReturn(self);
+
             [self _performScheduledExpirationCheck];
         });
     }