From 915fb2c2e37db143e3f5e62e59e94a008c0ffdbd Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Thu, 17 Oct 2024 18:13:21 -0700 Subject: [PATCH] [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 --- .../Framework/CHIP/MTRDevice_Concrete.mm | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 68b610cb25c7d7..2342cadfb253f5 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -389,6 +389,8 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle 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 @@ - (void)_ensureSubscriptionForExistingDelegates:(NSString *)reason 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 @@ - (void)_scheduleSubscriptionPoolWork:(dispatch_block_t)workBlock inNanoseconds: 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 @@ - (void)_handleResubscriptionNeededWithDelayOnDeviceQueue:(NSNumber *)resubscrip 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 @@ - (void)_doHandleSubscriptionReset:(NSNumber * _Nullable)retryDelay 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 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason mtr_weakify(self); dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast(NSEC_PER_SEC)), self.queue, ^{ mtr_strongify(self); - if (self != nil) { - [self _markDeviceAsUnreachableIfNeverSubscribed]; - } + VerifyOrReturn(self); + + [self _markDeviceAsUnreachableIfNeverSubscribed]; }); } @@ -3242,6 +3250,8 @@ - (void)_checkExpiredExpectedValues 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]; }); }