Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure we don't use the Matter dispatch queue when it's not running. #23859

Merged
merged 1 commit into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 58 additions & 27 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,24 @@ static void AddReadClientContainer(uint64_t deviceId, MTRReadClientContainer * c
[readClientContainersLock unlock];
}

static void PurgeReadClientContainers(uint64_t deviceId, dispatch_queue_t queue, void (^_Nullable completion)(void))
static void ReinstateReadClientList(NSMutableArray<MTRReadClientContainer *> * readClientList, NSNumber * key,
dispatch_queue_t queue, dispatch_block_t _Nullable completion)
{
[readClientContainersLock lock];
auto existingList = readClientContainers[key];
if (existingList) {
[existingList addObjectsFromArray:readClientList];
} else {
readClientContainers[key] = readClientList;
}
[readClientContainersLock unlock];
if (completion) {
dispatch_async(queue, completion);
}
}

static void PurgeReadClientContainers(
MTRDeviceController * controller, uint64_t deviceId, dispatch_queue_t queue, void (^_Nullable completion)(void))
{
InitializeReadClientContainers();

Expand All @@ -119,22 +136,28 @@ static void PurgeReadClientContainers(uint64_t deviceId, dispatch_queue_t queue,
[readClientContainersLock unlock];

// Destroy read clients in the work queue
dispatch_async(DeviceLayer::PlatformMgrImpl().GetWorkQueue(), ^{
for (MTRReadClientContainer * container in listToDelete) {
if (container.readClientPtr) {
Platform::Delete(container.readClientPtr);
container.readClientPtr = nullptr;
[controller
asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
for (MTRReadClientContainer * container in listToDelete) {
if (container.readClientPtr) {
Platform::Delete(container.readClientPtr);
container.readClientPtr = nullptr;
}
if (container.pathParams) {
Platform::Delete(container.pathParams);
container.pathParams = nullptr;
}
}
if (container.pathParams) {
Platform::Delete(container.pathParams);
container.pathParams = nullptr;
[listToDelete removeAllObjects];
if (completion) {
dispatch_async(queue, completion);
}
}
[listToDelete removeAllObjects];
if (completion) {
dispatch_async(queue, completion);
}
});
errorHandler:^(NSError * error) {
// Can't delete things. Just put them back, and hope we
// can delete them later.
ReinstateReadClientList(listToDelete, key, queue, completion);
}];
}

static void PurgeCompletedReadClientContainers(uint64_t deviceId)
Expand All @@ -157,7 +180,8 @@ static void PurgeCompletedReadClientContainers(uint64_t deviceId)

#ifdef DEBUG
// This function is for unit testing only. This function closes all read clients.
static void CauseReadClientFailure(uint64_t deviceId, dispatch_queue_t queue, void (^_Nullable completion)(void))
static void CauseReadClientFailure(
MTRDeviceController * controller, uint64_t deviceId, dispatch_queue_t queue, void (^_Nullable completion)(void))
{
InitializeReadClientContainers();

Expand All @@ -168,18 +192,23 @@ static void CauseReadClientFailure(uint64_t deviceId, dispatch_queue_t queue, vo
[readClientContainers removeObjectForKey:key];
[readClientContainersLock unlock];

dispatch_async(DeviceLayer::PlatformMgrImpl().GetWorkQueue(), ^{
for (MTRReadClientContainer * container in listToFail) {
// Send auto resubscribe request again by read clients, which must fail.
chip::app::ReadPrepareParams readParams;
if (container.readClientPtr) {
container.readClientPtr->SendAutoResubscribeRequest(std::move(readParams));
[controller
asyncDispatchToMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
for (MTRReadClientContainer * container in listToFail) {
// Send auto resubscribe request again by read clients, which must fail.
chip::app::ReadPrepareParams readParams;
if (container.readClientPtr) {
container.readClientPtr->SendAutoResubscribeRequest(std::move(readParams));
}
}
if (completion) {
dispatch_async(queue, completion);
}
}
if (completion) {
dispatch_async(queue, completion);
}
});
errorHandler:^(NSError * error) {
// Can't fail things. Just put them back.
ReinstateReadClientList(listToFail, key, queue, completion);
}];
}
#endif

Expand Down Expand Up @@ -321,6 +350,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
MTRClusterStateCacheContainer * container = weakPtr;
if (container) {
container.cppClusterStateCache = nullptr;
container.baseDevice = nil;
}
};
}
Expand Down Expand Up @@ -393,6 +423,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
clusterStateCacheContainer.cppClusterStateCache = clusterStateCache.get();
// ClusterStateCache will be deleted when OnDone is called.
callback->AdoptClusterStateCache(std::move(clusterStateCache));
clusterStateCacheContainer.baseDevice = self;
}
// Callback and ReadClient will be deleted when OnDone is called.
callback->AdoptReadClient(std::move(readClient));
Expand Down Expand Up @@ -1243,7 +1274,7 @@ - (void)deregisterReportHandlersWithQueue:(dispatch_queue_t)queue completion:(di
{
// This method must only be used for MTRDeviceOverXPC. However, for unit testing purpose, the method purges all read clients.
MTR_LOG_DEBUG("Unexpected call to deregister report handlers");
PurgeReadClientContainers(self.nodeID, queue, completion);
PurgeReadClientContainers(self.deviceController, self.nodeID, queue, completion);
}

namespace {
Expand Down Expand Up @@ -1389,7 +1420,7 @@ - (void)openCommissioningWindowWithSetupPasscode:(NSNumber *)setupPasscode
- (void)failSubscribers:(dispatch_queue_t)queue completion:(void (^)(void))completion
{
MTR_LOG_DEBUG("Causing failure in subscribers on purpose");
CauseReadClientFailure(self.nodeID, queue, completion);
CauseReadClientFailure(self.deviceController, self.nodeID, queue, completion);
}
#endif

Expand Down
35 changes: 22 additions & 13 deletions src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,29 +121,38 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
}

/**
* Run the given MTRLocalActionBlock on the Matter thread, then handle
* Try to run the given MTRLocalActionBlock on the Matter thread, if we have
* a device and it's attached to a running controller, then handle
* converting the value produced by the success callback to the right type
* so it can be passed to a callback of the type we're templated over.
*
* Does not attempt to establish any sessions to devices. Must not be used
* with any action blocks that need a session.
*/
void DispatchLocalAction(MTRLocalActionBlock _Nonnull action)
void DispatchLocalAction(MTRBaseDevice * _Nullable device, MTRLocalActionBlock _Nonnull action)
{
LogRequestStart();
if (!device) {
OnFailureFn(this, CHIP_ERROR_INCORRECT_STATE);
return;
}

// For now keep sync dispatch here.
dispatch_sync(chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue(), ^{
CHIP_ERROR err = action(mSuccess, mFailure);
if (err != CHIP_NO_ERROR) {
NSLog(@"Failure performing action. C++-mangled success callback type: '%s', error: %s", typeid(T).name(),
chip::ErrorStr(err));
LogRequestStart();

// Take the normal async error-reporting codepath. This will also
// handle cleaning us up properly.
OnFailureFn(this, err);
[device.deviceController
asyncDispatchToMatterQueue:^(chip::Controller::DeviceCommissioner *) {
CHIP_ERROR err = action(mSuccess, mFailure);
if (err != CHIP_NO_ERROR) {
NSLog(@"Failure performing action. C++-mangled success callback type: '%s', error: %s", typeid(T).name(),
chip::ErrorStr(err));

// Take the normal async error-reporting codepath. This will also
// handle cleaning us up properly.
OnFailureFn(this, err);
}
}
});
errorHandler:^(NSError * error) {
DispatchFailure(this, error);
}];
}

void ActionWithPASEDevice(MTRBaseDevice * device)
Expand Down
Loading