From da22b2d84d36c9a40776a1240bd760b6ea926dd3 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Mon, 24 Feb 2025 21:59:33 -0800 Subject: [PATCH 1/3] [Darwin] MTRDevice work blocks should only weakly reference self --- src/darwin/Framework/CHIP/MTRDevice_Concrete.mm | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index b2f39837eba14a..c29931fd34703f 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -708,8 +708,8 @@ - (void)_scheduleNextUpdate:(UInt64)nextUpdateInSeconds { mtr_weakify(self); dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (nextUpdateInSeconds * NSEC_PER_SEC)), self.queue, ^{ - MTR_LOG_DEBUG("%@ Timer expired, start Device Time Update", self); mtr_strongify(self); + MTR_LOG_DEBUG("%@ Timer expired, start Device Time Update", self); if (self) { [self _performScheduledTimeUpdate]; } else { @@ -3659,8 +3659,12 @@ - (void)invokeCommands:(NSArray *> *)c for (NSArray * commandGroup in [commands reverseObjectEnumerator]) { // We want to invoke all the commands in the group in order, propagating along the list of // current responses. Build up that linked list of command invokes via chaining the completions. + mtr_weakify(self); for (MTRCommandWithRequiredResponse * command in [commandGroup reverseObjectEnumerator]) { auto commandInvokeBlock = ^(BOOL allSucceededSoFar, NSArray * previousResponses) { + mtr_strongify(self); + VerifyOrReturn(self, MTR_LOG_DEBUG("invokeCommands commandInvokeBlock called back with nil MTRDevice")); + [self invokeCommandWithEndpointID:command.path.endpoint clusterID:command.path.cluster commandID:command.path.command @@ -3669,6 +3673,8 @@ - (void)invokeCommands:(NSArray *> *)c expectedValueInterval:nil queue:self.queue completion:^(NSArray *> * responses, NSError * error) { + mtr_strongify(self); + VerifyOrReturn(self, MTR_LOG_DEBUG("invokeCommands invokeCommandWithEndpointID completion called back with nil MTRDevice")); if (error != nil) { nextCompletion(NO, [previousResponses arrayByAddingObject:@ { MTRCommandPathKey : command.path, @@ -3701,6 +3707,9 @@ - (void)invokeCommands:(NSArray *> *)c } auto commandGroupInvokeBlock = ^(BOOL allSucceededSoFar, NSArray * previousResponses) { + mtr_strongify(self); + VerifyOrReturn(self, MTR_LOG_DEBUG("invokeCommands commandGroupInvokeBlock called back with nil MTRDevice")); + if (allSucceededSoFar == NO) { // Don't start a new command group if something failed in the // previous one. Note that we might be running on self.queue here, so make sure we From ad0179499dff19e0f356edb642ceee13e981117c Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Tue, 25 Feb 2025 10:26:00 -0800 Subject: [PATCH 2/3] Have invokeCommands call completion even when object is gone --- src/darwin/Framework/CHIP/MTRDevice_Concrete.mm | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index c29931fd34703f..6cf616423c17ed 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3663,7 +3663,15 @@ - (void)invokeCommands:(NSArray *> *)c for (MTRCommandWithRequiredResponse * command in [commandGroup reverseObjectEnumerator]) { auto commandInvokeBlock = ^(BOOL allSucceededSoFar, NSArray * previousResponses) { mtr_strongify(self); - VerifyOrReturn(self, MTR_LOG_DEBUG("invokeCommands commandInvokeBlock called back with nil MTRDevice")); + if (!self) { + MTR_LOG_DEBUG("invokeCommands commandInvokeBlock called back with nil MTRDevice"); + // Object is gone, but make sure next completion gets called appropriately. + nextCompletion(NO, [previousResponses arrayByAddingObject:@{ + MTRCommandPathKey : command.path, + MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL], + }]); + return; + } [self invokeCommandWithEndpointID:command.path.endpoint clusterID:command.path.cluster From 9568ffb098dbbd3193001c13a8198f31701e9563 Mon Sep 17 00:00:00 2001 From: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> Date: Tue, 25 Feb 2025 10:47:52 -0800 Subject: [PATCH 3/3] Revert "Have invokeCommands call completion even when object is gone" This reverts commit ad0179499dff19e0f356edb642ceee13e981117c. --- src/darwin/Framework/CHIP/MTRDevice_Concrete.mm | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index 6cf616423c17ed..c29931fd34703f 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3663,15 +3663,7 @@ - (void)invokeCommands:(NSArray *> *)c for (MTRCommandWithRequiredResponse * command in [commandGroup reverseObjectEnumerator]) { auto commandInvokeBlock = ^(BOOL allSucceededSoFar, NSArray * previousResponses) { mtr_strongify(self); - if (!self) { - MTR_LOG_DEBUG("invokeCommands commandInvokeBlock called back with nil MTRDevice"); - // Object is gone, but make sure next completion gets called appropriately. - nextCompletion(NO, [previousResponses arrayByAddingObject:@{ - MTRCommandPathKey : command.path, - MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL], - }]); - return; - } + VerifyOrReturn(self, MTR_LOG_DEBUG("invokeCommands commandInvokeBlock called back with nil MTRDevice")); [self invokeCommandWithEndpointID:command.path.endpoint clusterID:command.path.cluster