Skip to content

Commit

Permalink
Make sure to not dispatch to shut-down queues in MTROTAProviderDelega…
Browse files Browse the repository at this point in the history
…teBridge. (#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
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Dec 19, 2023
1 parent a3e0375 commit 1553330
Show file tree
Hide file tree
Showing 4 changed files with 239 additions and 123 deletions.
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ MTR_NEWLY_AVAILABLE
/*
* OTA Provider delegate to be called when an OTA Requestor is requesting a software update.
* Defaults to nil.
*
* Calls to this delegate can happen on an arbitrary thread, but will not happen
* concurrently.
*/
@property (nonatomic, strong, nullable) id<MTROTAProviderDelegate> otaProviderDelegate;

Expand Down
19 changes: 13 additions & 6 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,11 @@ - (MTRDeviceController * _Nullable)maybeInitializeOTAProvider:(MTRDeviceControll
VerifyOrReturnValue(_otaProviderDelegateBridge != nil, controller);
VerifyOrReturnValue([_controllers count] == 1, controller);

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

if ([_controllers count] == 0) {
if (_otaProviderDelegateBridge) {
_otaProviderDelegateBridge->Shutdown();
}

// That was our last controller. Stop the event loop before it
// shuts down, because shutdown of the last controller will tear
// down most of the world.
DeviceLayer::PlatformMgrImpl().StopEventLoopTask();

if (_otaProviderDelegateBridge) {
_otaProviderDelegateBridge->Shutdown();
}

[controller shutDownCppController];
} else {
// Do the controller shutdown on the Matter work queue.
dispatch_sync(_chipWorkQueue, ^{
if (_otaProviderDelegateBridge) {
_otaProviderDelegateBridge->ControllerShuttingDown(controller);
}

[controller shutDownCppController];
});
}
Expand Down
9 changes: 8 additions & 1 deletion src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
~MTROTAProviderDelegateBridge();

CHIP_ERROR Init(chip::System::Layer * systemLayer, chip::Messaging::ExchangeManager * exchangeManager);

// Shutdown must be called after the event loop has been stopped, since it
// touches Matter objects.
void Shutdown();

// ControllerShuttingDown must be called on the Matter work queue, since it
// touches Matter objects.
void ControllerShuttingDown(MTRDeviceController * controller);

void HandleQueryImage(
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override;
Expand Down Expand Up @@ -60,7 +67,7 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
MTROTASoftwareUpdateProviderClusterNotifyUpdateAppliedParams * commandParams);

_Nullable id<MTROTAProviderDelegate> mDelegate;
dispatch_queue_t mWorkQueue;
dispatch_queue_t mDelegateNotificationQueue;
};

NS_ASSUME_NONNULL_END
Loading

0 comments on commit 1553330

Please sign in to comment.