[Darwin tests] Fix testAnySharedRemoteController and testReadClusterStateCacheFailure (#23663)
This is a re-landing of PR #22838.
* [Darwin Tests] Fix testAnySharedRemoteController
* [Darwin Tests] Fix testReadClusterStateCacheFailure
Co-authored-by: Vivien Nicolas <vnicolas@apple.com>
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC.m b/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC.m
index f8f2e18..929e479 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC.m
+++ b/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC.m
@@ -158,7 +158,8 @@
// under here if we don't have a controller id aleady, so make sure we do
// that.
[self fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
+ completion:^(id _Nullable controllerID, MTRDeviceControllerXPCProxyHandle * _Nullable handle,
+ NSError * _Nullable error) {
if (error != nil) {
completionHandler(nil, error);
return;
@@ -172,6 +173,9 @@
- (void)fetchControllerIdWithQueue:(dispatch_queue_t)queue completion:(MTRFetchControllerIDCompletion)completion
{
+ // Capture the proxy handle so that it won't be released prior to block call.
+ __block MTRDeviceControllerXPCProxyHandle * handleRetainer = nil;
+
dispatch_async(_workQueue, ^{
dispatch_group_t group = dispatch_group_create();
if (!self.controllerID) {
@@ -184,10 +188,9 @@
MTR_LOG_ERROR("Failed to fetch any shared remote controller");
} else {
self.controllerID = controller;
+ handleRetainer = handle;
}
dispatch_group_leave(group);
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
}];
} else {
MTR_LOG_ERROR("XPC disconnected while retrieving any shared remote controller");
@@ -197,9 +200,9 @@
}
dispatch_group_notify(group, queue, ^{
if (self.controllerID) {
- completion(self.controllerID, nil);
+ completion(self.controllerID, handleRetainer, nil);
} else {
- completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
+ completion(nil, nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
}
});
});
diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC_Internal.h b/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC_Internal.h
index d772dab..ccbc2f2 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC_Internal.h
+++ b/src/darwin/Framework/CHIP/MTRDeviceControllerOverXPC_Internal.h
@@ -20,7 +20,8 @@
NS_ASSUME_NONNULL_BEGIN
-typedef void (^MTRFetchControllerIDCompletion)(id _Nullable controllerID, NSError * _Nullable error);
+typedef void (^MTRFetchControllerIDCompletion)(
+ id _Nullable controllerID, MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error);
@interface MTRDeviceControllerOverXPC ()
diff --git a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.m b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.m
index a780929..a746d0a 100644
--- a/src/darwin/Framework/CHIP/MTRDeviceOverXPC.m
+++ b/src/darwin/Framework/CHIP/MTRDeviceOverXPC.m
@@ -27,6 +27,8 @@
NS_ASSUME_NONNULL_BEGIN
+typedef void (^MTRFetchProxyHandleCompletion)(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error);
+
@interface MTRDeviceOverXPC ()
@property (nonatomic, strong, readonly, nullable) id<NSCopying> controllerID;
@@ -34,6 +36,8 @@
@property (nonatomic, readonly) NSNumber * nodeID;
@property (nonatomic, strong, readonly) MTRDeviceControllerXPCConnection * xpcConnection;
+- (void)fetchProxyHandleWithQueue:(dispatch_queue_t)queue completion:(MTRFetchProxyHandleCompletion)completion;
+
@end
@implementation MTRDeviceOverXPC
@@ -60,54 +64,37 @@
{
MTR_LOG_DEBUG("Subscribing all attributes... Note that attributeReportHandler, eventReportHandler, and resubscriptionScheduled "
"are not supported.");
- __auto_type workBlock = ^{
+
+ __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ if (error != nil) {
+ errorHandler(error);
+ return;
+ }
+
if (clusterStateCacheContainer) {
[clusterStateCacheContainer setXPCConnection:self->_xpcConnection controllerID:self.controllerID deviceID:self.nodeID];
}
- [self->_xpcConnection getProxyHandleWithCompletion:^(
- dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
- if (handle) {
- [handle.proxy subscribeWithController:self.controllerID
- nodeId:self.nodeID.unsignedLongLongValue
- minInterval:params.minInterval
- maxInterval:params.maxInterval
- params:[MTRDeviceController encodeXPCSubscribeParams:params]
- shouldCache:(clusterStateCacheContainer != nil)
- completion:^(NSError * _Nullable error) {
- dispatch_async(queue, ^{
- if (error) {
- errorHandler(error);
- } else {
- subscriptionEstablishedHandler();
- }
- });
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
- }];
- } else {
- MTR_LOG_ERROR("Failed to obtain XPC connection to write attribute");
- dispatch_async(queue, ^{
- errorHandler([NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
- });
- }
- }];
+
+ [handle.proxy subscribeWithController:self.controllerID
+ nodeId:self.nodeID.unsignedLongLongValue
+ minInterval:params.minInterval
+ maxInterval:params.maxInterval
+ params:[MTRDeviceController encodeXPCSubscribeParams:params]
+ shouldCache:(clusterStateCacheContainer != nil)
+ completion:^(NSError * _Nullable error) {
+ dispatch_async(queue, ^{
+ if (error) {
+ errorHandler(error);
+ } else {
+ subscriptionEstablishedHandler();
+ }
+ });
+ __auto_type handleRetainer = handle;
+ (void) handleRetainer;
+ }];
};
- if (self.controllerID != nil) {
- workBlock();
- } else {
- [self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
- if (error != nil) {
- // We're already running on the right queue.
- errorHandler(error);
- return;
- }
-
- self->_controllerID = controllerID;
- workBlock();
- }];
- }
+ [self fetchProxyHandleWithQueue:queue completion:workBlock];
}
- (void)readAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
@@ -118,50 +105,32 @@
completion:(MTRDeviceResponseHandler)completion
{
MTR_LOG_DEBUG("Reading attribute ...");
- __auto_type workBlock = ^{
- [self->_xpcConnection getProxyHandleWithCompletion:^(
- dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
- if (handle) {
- [handle.proxy readAttributeWithController:self.controllerID
- nodeId:self.nodeID.unsignedLongLongValue
- endpointId:endpointID
- clusterId:clusterID
- attributeId:attributeID
- params:[MTRDeviceController encodeXPCReadParams:params]
- completion:^(id _Nullable values, NSError * _Nullable error) {
- dispatch_async(queue, ^{
- MTR_LOG_DEBUG("Attribute read");
- completion([MTRDeviceController decodeXPCResponseValues:values], error);
- // The following captures the proxy handle in the closure so that the
- // handle won't be released prior to block call.
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
- });
- }];
- } else {
- dispatch_async(queue, ^{
- MTR_LOG_ERROR("Failed to obtain XPC connection to read attribute");
- completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
- });
- }
- }];
+
+ __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ if (error != nil) {
+ completion(nil, error);
+ return;
+ }
+
+ [handle.proxy readAttributeWithController:self.controllerID
+ nodeId:self.nodeID.unsignedLongLongValue
+ endpointId:endpointID
+ clusterId:clusterID
+ attributeId:attributeID
+ params:[MTRDeviceController encodeXPCReadParams:params]
+ completion:^(id _Nullable values, NSError * _Nullable error) {
+ dispatch_async(queue, ^{
+ MTR_LOG_DEBUG("Attribute read");
+ completion([MTRDeviceController decodeXPCResponseValues:values], error);
+ // The following captures the proxy handle in the closure so that the
+ // handle won't be released prior to block call.
+ __auto_type handleRetainer = handle;
+ (void) handleRetainer;
+ });
+ }];
};
- if (self.controllerID != nil) {
- workBlock();
- } else {
- [self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
- if (error != nil) {
- // We're already running on the right queue.
- completion(nil, error);
- return;
- }
-
- self->_controllerID = controllerID;
- workBlock();
- }];
- }
+ [self fetchProxyHandleWithQueue:queue completion:workBlock];
}
- (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
@@ -173,51 +142,33 @@
completion:(MTRDeviceResponseHandler)completion
{
MTR_LOG_DEBUG("Writing attribute ...");
- __auto_type workBlock = ^{
- [self->_xpcConnection getProxyHandleWithCompletion:^(
- dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
- if (handle) {
- [handle.proxy writeAttributeWithController:self.controllerID
- nodeId:self.nodeID.unsignedLongLongValue
- endpointId:endpointID
- clusterId:clusterID
- attributeId:attributeID
- value:value
- timedWriteTimeout:timeoutMs
- completion:^(id _Nullable values, NSError * _Nullable error) {
- dispatch_async(queue, ^{
- MTR_LOG_DEBUG("Attribute written");
- completion([MTRDeviceController decodeXPCResponseValues:values], error);
- // The following captures the proxy handle in the closure so that the
- // handle won't be released prior to block call.
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
- });
- }];
- } else {
- dispatch_async(queue, ^{
- MTR_LOG_ERROR("Failed to obtain XPC connection to write attribute");
- completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
- });
- }
- }];
+
+ __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ if (error != nil) {
+ completion(nil, error);
+ return;
+ }
+
+ [handle.proxy writeAttributeWithController:self.controllerID
+ nodeId:self.nodeID.unsignedLongLongValue
+ endpointId:endpointID
+ clusterId:clusterID
+ attributeId:attributeID
+ value:value
+ timedWriteTimeout:timeoutMs
+ completion:^(id _Nullable values, NSError * _Nullable error) {
+ dispatch_async(queue, ^{
+ MTR_LOG_DEBUG("Attribute written");
+ completion([MTRDeviceController decodeXPCResponseValues:values], error);
+ // The following captures the proxy handle in the closure so that the
+ // handle won't be released prior to block call.
+ __auto_type handleRetainer = handle;
+ (void) handleRetainer;
+ });
+ }];
};
- if (self.controllerID != nil) {
- workBlock();
- } else {
- [self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
- if (error != nil) {
- // We're already running on the right queue.
- completion(nil, error);
- return;
- }
-
- self->_controllerID = controllerID;
- workBlock();
- }];
- }
+ [self fetchProxyHandleWithQueue:queue completion:workBlock];
}
- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
@@ -229,51 +180,33 @@
completion:(MTRDeviceResponseHandler)completion
{
MTR_LOG_DEBUG("Invoking command ...");
- __auto_type workBlock = ^{
- [self->_xpcConnection getProxyHandleWithCompletion:^(
- dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
- if (handle) {
- [handle.proxy invokeCommandWithController:self.controllerID
- nodeId:self.nodeID.unsignedLongLongValue
- endpointId:endpointID
- clusterId:clusterID
- commandId:commandID
- fields:commandFields
- timedInvokeTimeout:timeoutMs
- completion:^(id _Nullable values, NSError * _Nullable error) {
- dispatch_async(queue, ^{
- MTR_LOG_DEBUG("Command invoked");
- completion([MTRDeviceController decodeXPCResponseValues:values], error);
- // The following captures the proxy handle in the closure so that the
- // handle won't be released prior to block call.
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
- });
- }];
- } else {
- dispatch_async(queue, ^{
- MTR_LOG_ERROR("Failed to obtain XPC connection to invoke command");
- completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
- });
- }
- }];
+
+ __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ if (error != nil) {
+ completion(nil, error);
+ return;
+ }
+
+ [handle.proxy invokeCommandWithController:self.controllerID
+ nodeId:self.nodeID.unsignedLongLongValue
+ endpointId:endpointID
+ clusterId:clusterID
+ commandId:commandID
+ fields:commandFields
+ timedInvokeTimeout:timeoutMs
+ completion:^(id _Nullable values, NSError * _Nullable error) {
+ dispatch_async(queue, ^{
+ MTR_LOG_DEBUG("Command invoked");
+ completion([MTRDeviceController decodeXPCResponseValues:values], error);
+ // The following captures the proxy handle in the closure so that the
+ // handle won't be released prior to block call.
+ __auto_type handleRetainer = handle;
+ (void) handleRetainer;
+ });
+ }];
};
- if (self.controllerID != nil) {
- workBlock();
- } else {
- [self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
- if (error != nil) {
- // We're already running on the right queue.
- completion(nil, error);
- return;
- }
-
- self->_controllerID = controllerID;
- workBlock();
- }];
- }
+ [self fetchProxyHandleWithQueue:queue completion:workBlock];
}
- (void)subscribeToAttributesWithEndpointID:(NSNumber * _Nullable)endpointID
@@ -285,92 +218,72 @@
subscriptionEstablished:(void (^_Nullable)(void))subscriptionEstablishedHandler
{
MTR_LOG_DEBUG("Subscribing attribute ...");
- __auto_type workBlock = ^{
- [self->_xpcConnection getProxyHandleWithCompletion:^(
- dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
- if (handle) {
- MTR_LOG_DEBUG("Setup report handler");
- [self.xpcConnection
- registerReportHandlerWithController:self.controllerID
- nodeID:self.nodeID
- handler:^(id _Nullable values, NSError * _Nullable error) {
- if (values && ![values isKindOfClass:[NSArray class]]) {
- MTR_LOG_ERROR("Unsupported report format");
- return;
- }
- if (!values) {
- MTR_LOG_DEBUG("Error report received");
- dispatch_async(queue, ^{
- reportHandler(values, error);
- });
- return;
- }
- __auto_type decodedValues =
- [MTRDeviceController decodeXPCResponseValues:values];
- NSMutableArray<NSDictionary<NSString *, id> *> * filteredValues =
- [NSMutableArray arrayWithCapacity:[decodedValues count]];
- for (NSDictionary<NSString *, id> * decodedValue in decodedValues) {
- MTRAttributePath * attributePath = decodedValue[MTRAttributePathKey];
- if ((endpointID == nil ||
- [attributePath.endpoint isEqualToNumber:endpointID])
- && (clusterID == nil ||
- [attributePath.cluster isEqualToNumber:clusterID])
- && (attributeID == nil ||
- [attributePath.attribute isEqualToNumber:attributeID])) {
- [filteredValues addObject:decodedValue];
- }
- }
- if ([filteredValues count] > 0) {
- MTR_LOG_DEBUG("Report received");
- dispatch_async(queue, ^{
- reportHandler(filteredValues, error);
- });
- }
- }];
- [handle.proxy subscribeAttributeWithController:self.controllerID
- nodeId:self.nodeID.unsignedLongLongValue
- endpointId:endpointID
- clusterId:clusterID
- attributeId:attributeID
- minInterval:params.minInterval
- maxInterval:params.maxInterval
- params:[MTRDeviceController encodeXPCSubscribeParams:params]
- establishedHandler:^{
+ __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ MTR_LOG_DEBUG("Setup report handler");
+
+ if (error != nil) {
+ subscriptionEstablishedHandler();
+ reportHandler(nil, error);
+ return;
+ }
+
+ [self.xpcConnection
+ registerReportHandlerWithController:self.controllerID
+ nodeID:self.nodeID
+ handler:^(id _Nullable values, NSError * _Nullable error) {
+ if (values && ![values isKindOfClass:[NSArray class]]) {
+ MTR_LOG_ERROR("Unsupported report format");
+ return;
+ }
+ if (!values) {
+ MTR_LOG_DEBUG("Error report received");
dispatch_async(queue, ^{
- MTR_LOG_DEBUG("Subscription established");
- subscriptionEstablishedHandler();
- // The following captures the proxy handle in the closure so that the handle
- // won't be released prior to block call.
- __auto_type handleRetainer = handle;
- (void) handleRetainer;
+ reportHandler(values, error);
});
- }];
- } else {
- dispatch_async(queue, ^{
- MTR_LOG_ERROR("Failed to obtain XPC connection to subscribe to attribute");
- subscriptionEstablishedHandler();
- reportHandler(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
- });
- }
- }];
+ return;
+ }
+ __auto_type decodedValues = [MTRDeviceController decodeXPCResponseValues:values];
+ NSMutableArray<NSDictionary<NSString *, id> *> * filteredValues =
+ [NSMutableArray arrayWithCapacity:[decodedValues count]];
+ for (NSDictionary<NSString *, id> * decodedValue in decodedValues) {
+ MTRAttributePath * attributePath = decodedValue[MTRAttributePathKey];
+ if ((endpointID == nil || [attributePath.endpoint isEqualToNumber:endpointID])
+ && (clusterID == nil || [attributePath.cluster isEqualToNumber:clusterID])
+ && (attributeID == nil ||
+ [attributePath.attribute isEqualToNumber:attributeID])) {
+ [filteredValues addObject:decodedValue];
+ }
+ }
+ if ([filteredValues count] > 0) {
+ MTR_LOG_DEBUG("Report received");
+ dispatch_async(queue, ^{
+ reportHandler(filteredValues, error);
+ });
+ }
+ }];
+
+ [handle.proxy subscribeAttributeWithController:self.controllerID
+ nodeId:self.nodeID.unsignedLongLongValue
+ endpointId:endpointID
+ clusterId:clusterID
+ attributeId:attributeID
+ minInterval:params.minInterval
+ maxInterval:params.maxInterval
+ params:[MTRDeviceController encodeXPCSubscribeParams:params]
+ establishedHandler:^{
+ dispatch_async(queue, ^{
+ MTR_LOG_DEBUG("Subscription established");
+ subscriptionEstablishedHandler();
+ // The following captures the proxy handle in the closure so that the handle
+ // won't be released prior to block call.
+ __auto_type handleRetainer = handle;
+ (void) handleRetainer;
+ });
+ }];
};
- if (self.controllerID != nil) {
- workBlock();
- } else {
- [self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
- if (error != nil) {
- // We're already running on the right queue.
- reportHandler(nil, error);
- return;
- }
-
- self->_controllerID = controllerID;
- workBlock();
- }];
- }
+ [self fetchProxyHandleWithQueue:queue completion:workBlock];
}
- (void)deregisterReportHandlersWithQueue:(dispatch_queue_t)queue completion:(void (^)(void))completion
@@ -388,7 +301,8 @@
workBlock();
} else {
[self.controller fetchControllerIdWithQueue:queue
- completion:^(id _Nullable controllerID, NSError * _Nullable error) {
+ completion:^(id _Nullable controllerID,
+ MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
if (error != nil) {
// We're already running on the right queue.
completion();
@@ -413,6 +327,35 @@
});
}
+- (void)fetchProxyHandleWithQueue:(dispatch_queue_t)queue completion:(MTRFetchProxyHandleCompletion)completion
+{
+ if (self.controllerID != nil) {
+ [self->_xpcConnection getProxyHandleWithCompletion:^(
+ dispatch_queue_t _Nonnull proxyQueue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
+ dispatch_async(queue, ^{
+ if (handle == nil) {
+ MTR_LOG_ERROR("Failed to obtain XPC connection");
+ completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
+ } else {
+ completion(handle, nil);
+ }
+ });
+ }];
+ } else {
+ [self.controller fetchControllerIdWithQueue:queue
+ completion:^(id _Nullable controllerID,
+ MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+ // We're already running on the right queue.
+ if (error != nil) {
+ completion(nil, error);
+ } else {
+ self->_controllerID = controllerID;
+ completion(handle, nil);
+ }
+ }];
+ }
+}
+
@end
NS_ASSUME_NONNULL_END
diff --git a/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m b/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m
index b358f7c..aac3da2 100644
--- a/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m
@@ -2349,6 +2349,7 @@
completion(nil, myError);
};
+ _xpcDisconnectExpectation = [self expectationWithDescription:@"XPC Disconnected"];
[clusterStateCacheContainer subscribeWithDeviceController:_remoteDeviceController
deviceID:@(myNodeId)
params:nil
@@ -2358,7 +2359,7 @@
XCTAssertNil(error);
[subscribeExpectation fulfill];
}];
- [self waitForExpectations:@[ subscribeExpectation ] timeout:kTimeoutInSeconds];
+ [self waitForExpectations:@[ subscribeExpectation, _xpcDisconnectExpectation ] timeout:kTimeoutInSeconds];
_xpcDisconnectExpectation = [self expectationWithDescription:@"XPC Disconnected"];
[clusterStateCacheContainer