Skip to content

Commit

Permalink
[Darwin] Some MTRDevice mtr_strongify sites need to guard against nil…
Browse files Browse the repository at this point in the history
… self (project-chip#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
  • Loading branch information
jtung-apple authored Oct 18, 2024
1 parent 3a6c545 commit 915fb2c
Showing 1 changed file with 18 additions and 8 deletions.
26 changes: 18 additions & 8 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}];
Expand Down Expand Up @@ -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]];
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"];
Expand Down Expand Up @@ -2386,9 +2394,9 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
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];
});
}

Expand Down Expand Up @@ -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];
});
}
Expand Down

0 comments on commit 915fb2c

Please sign in to comment.