Skip to content

Commit 9940030

Browse files
bzbarsky-applepull[bot]
authored andcommitted
Make sure to not dispatch to shut-down queues in MTROTAProviderDelegateBridge. (#23841)
* Make sure we call our delegate callbacks on a queue different from the Matter work queue. * Assert that we are on the Matter work queue in all the places where we should be. * Make sure to dispatch to the Matter work queue via the controller we were dealing with, so dispatch does not happen if that controller has shut down. * Reset OTA transfers when the controller the transfer is associated with shuts down. * Ensure that async callbacks for a stale transfer don't affect a current transfer. Fixes #22541
1 parent 2e96220 commit 9940030

File tree

4 files changed

+239
-123
lines changed

4 files changed

+239
-123
lines changed

src/darwin/Framework/CHIP/MTRDeviceControllerFactory.h

+3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ MTR_NEWLY_AVAILABLE
4444
/*
4545
* OTA Provider delegate to be called when an OTA Requestor is requesting a software update.
4646
* Defaults to nil.
47+
*
48+
* Calls to this delegate can happen on an arbitrary thread, but will not happen
49+
* concurrently.
4750
*/
4851
@property (nonatomic, strong, nullable) id<MTROTAProviderDelegate> otaProviderDelegate;
4952

src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm

+13-6
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,11 @@ - (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceControll
674674
VerifyOrReturnValue(_otaProviderDelegateBridge != nil, controller);
675675
VerifyOrReturnValue([_controllers count] == 1, controller);
676676

677-
auto systemState = _controllerFactory->GetSystemState();
678-
CHIP_ERROR err = _otaProviderDelegateBridge->Init(systemState->SystemLayer(), systemState->ExchangeMgr());
677+
__block CHIP_ERROR err;
678+
dispatch_sync(_chipWorkQueue, ^{
679+
auto systemState = _controllerFactory->GetSystemState();
680+
err = _otaProviderDelegateBridge->Init(systemState->SystemLayer(), systemState->ExchangeMgr());
681+
});
679682
if (CHIP_NO_ERROR != err) {
680683
MTR_LOG_ERROR("Failed to init provider delegate bridge: %" CHIP_ERROR_FORMAT, err.Format());
681684
[controller shutdown];
@@ -711,19 +714,23 @@ - (void)controllerShuttingDown:(MTRDeviceController *)controller
711714
[_controllers removeObject:controller];
712715

713716
if ([_controllers count] == 0) {
714-
if (_otaProviderDelegateBridge) {
715-
_otaProviderDelegateBridge->Shutdown();
716-
}
717-
718717
// That was our last controller. Stop the event loop before it
719718
// shuts down, because shutdown of the last controller will tear
720719
// down most of the world.
721720
DeviceLayer::PlatformMgrImpl().StopEventLoopTask();
722721

722+
if (_otaProviderDelegateBridge) {
723+
_otaProviderDelegateBridge->Shutdown();
724+
}
725+
723726
[controller shutDownCppController];
724727
} else {
725728
// Do the controller shutdown on the Matter work queue.
726729
dispatch_sync(_chipWorkQueue, ^{
730+
if (_otaProviderDelegateBridge) {
731+
_otaProviderDelegateBridge->ControllerShuttingDown(controller);
732+
}
733+
727734
[controller shutDownCppController];
728735
});
729736
}

src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,15 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
2828
~MTROTAProviderDelegateBridge();
2929

3030
CHIP_ERROR Init(chip::System::Layer * systemLayer, chip::Messaging::ExchangeManager * exchangeManager);
31+
32+
// Shutdown must be called after the event loop has been stopped, since it
33+
// touches Matter objects.
3134
void Shutdown();
3235

36+
// ControllerShuttingDown must be called on the Matter work queue, since it
37+
// touches Matter objects.
38+
void ControllerShuttingDown(MTRDeviceController * controller);
39+
3340
void HandleQueryImage(
3441
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
3542
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override;
@@ -60,7 +67,7 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
6067
MTROTASoftwareUpdateProviderClusterNotifyUpdateAppliedParams * commandParams);
6168

6269
_Nullable id<MTROTAProviderDelegate> mDelegate;
63-
dispatch_queue_t mWorkQueue;
70+
dispatch_queue_t mDelegateNotificationQueue;
6471
};
6572

6673
NS_ASSUME_NONNULL_END

0 commit comments

Comments
 (0)