diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 356e607ad86d4b..0712e779c525e6 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -91,12 +91,18 @@ NSString * const MTREventIsHistoricalKey = @"eventIsHistorical"; class MTRDataValueDictionaryCallbackBridge; +class MTRDataValueDictionaryDecodableType; +template +class BufferedReadClientCallback; @interface MTRReadClientContainer : NSObject @property (nonatomic, readwrite) app::ReadClient * readClientPtr; +@property (nonatomic, readwrite) BufferedReadClientCallback * callback; @property (nonatomic, readwrite) app::AttributePathParams * pathParams; @property (nonatomic, readwrite) app::EventPathParams * eventPathParams; @property (nonatomic, readwrite) uint64_t deviceID; + +- (void)cleanup; - (void)onDone; @end @@ -157,22 +163,7 @@ static void PurgeReadClientContainers( [controller asyncDispatchToMatterQueue:^() { for (MTRReadClientContainer * container in listToDelete) { - if (container.readClientPtr) { - Platform::Delete(container.readClientPtr); - container.readClientPtr = nullptr; - } - if (container.pathParams) { - static_assert(std::is_trivially_destructible::value, - "AttributePathParams destructors won't get run"); - Platform::MemoryFree(container.pathParams); - container.pathParams = nullptr; - } - if (container.eventPathParams) { - static_assert( - std::is_trivially_destructible::value, "EventPathParams destructors won't get run"); - Platform::MemoryFree(container.eventPathParams); - container.eventPathParams = nullptr; - } + [container cleanup]; } [listToDelete removeAllObjects]; if (completion) { @@ -204,47 +195,13 @@ static void PurgeCompletedReadClientContainers(uint64_t deviceId) [readClientContainersLock unlock]; } -#ifdef DEBUG -// This function is for unit testing only. This function closes all read clients. -static void CauseReadClientFailure( - MTRDeviceController * controller, uint64_t deviceId, dispatch_queue_t queue, void (^_Nullable completion)(void)) -{ - InitializeReadClientContainers(); - - NSMutableArray * listToFail; - NSNumber * key = [NSNumber numberWithUnsignedLongLong:deviceId]; - [readClientContainersLock lock]; - listToFail = readClientContainers[key]; - [readClientContainers removeObjectForKey:key]; - [readClientContainersLock unlock]; - - [controller - asyncDispatchToMatterQueue:^() { - 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); - } - } - errorHandler:^(NSError * error) { - // Can't fail things. Just put them back. - ReinstateReadClientList(listToFail, key, queue, completion); - }]; -} -#endif - static bool CheckMemberOfType(NSDictionary * responseValue, NSString * memberName, Class expectedClass, NSString * errorMessage, NSError * __autoreleasing * error); static void LogStringAndReturnError(NSString * errorStr, CHIP_ERROR errorCode, NSError * __autoreleasing * error); static void LogStringAndReturnError(NSString * errorStr, MTRErrorCode errorCode, NSError * __autoreleasing * error); @implementation MTRReadClientContainer -- (void)onDone +- (void)cleanup { if (_readClientPtr) { Platform::Delete(_readClientPtr); @@ -260,26 +217,19 @@ - (void)onDone Platform::MemoryFree(_eventPathParams); _eventPathParams = nullptr; } - PurgeCompletedReadClientContainers(_deviceID); + if (_callback) { + Platform::Delete(_callback); + _callback = nullptr; + } } -- (void)dealloc +- (void)onDone { - if (_readClientPtr) { - Platform::Delete(_readClientPtr); - _readClientPtr = nullptr; - } - if (_pathParams) { - static_assert(std::is_trivially_destructible::value, "AttributePathParams destructors won't get run"); - Platform::MemoryFree(_pathParams); - _pathParams = nullptr; - } - if (_eventPathParams) { - static_assert(std::is_trivially_destructible::value, "EventPathParams destructors won't get run"); - Platform::MemoryFree(_eventPathParams); - _eventPathParams = nullptr; - } + [self cleanup]; + + PurgeCompletedReadClientContainers(_deviceID); } + @end @implementation MTRBaseDevice @@ -1730,6 +1680,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl dispatch_async(queue, ^{ reportHandler(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY]); }); + [container cleanup]; return; } for (MTRAttributeRequestPath * attribute in attributes) { @@ -1744,6 +1695,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl dispatch_async(queue, ^{ reportHandler(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_NO_MEMORY]); }); + [container cleanup]; return; } for (MTREventRequestPath * event in events) { @@ -1763,9 +1715,6 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl auto onDone = [container](BufferedReadClientCallback * callback) { [container onDone]; - // Make sure we delete callback last, because doing that actually destroys our - // lambda, so we can't access captured values after that. - chip::Platform::Delete(callback); }; auto callback = chip::Platform::MakeUnique>( @@ -1775,6 +1724,9 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl auto readClient = Platform::New( engine, exchangeManager, callback->GetBufferedCallback(), chip::app::ReadClient::InteractionType::Subscribe); + container.readClientPtr = readClient; + container.callback = callback.release(); + if (!params.resubscribeAutomatically) { err = readClient->SendRequest(readParams); } else { @@ -1785,23 +1737,12 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl dispatch_async(queue, ^{ reportHandler(nil, [MTRError errorForCHIPErrorCode:err]); }); - Platform::Delete(readClient); - if (container.pathParams != nullptr) { - Platform::MemoryFree(container.pathParams); - } - - if (container.eventPathParams != nullptr) { - Platform::MemoryFree(container.eventPathParams); - } - container.pathParams = nullptr; - container.eventPathParams = nullptr; + [container cleanup]; return; } // Read clients will be purged when deregistered. - container.readClientPtr = readClient; AddReadClientContainer(container.deviceID, container); - callback.release(); }]; } @@ -2024,13 +1965,6 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator } #ifdef DEBUG -// This method is for unit testing only -- (void)failSubscribers:(dispatch_queue_t)queue completion:(void (^)(void))completion -{ - MTR_LOG_DEBUG("Causing failure in subscribers on purpose"); - CauseReadClientFailure(self.deviceController, self.nodeID, queue, completion); -} - // The following method is for unit testing purpose only + (id)CHIPEncodeAndDecodeNSObject:(id)object { diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index 2dc01461826b40..7a7a3d82043699 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -1071,8 +1071,8 @@ - (void)test012_SubscriptionError XCTAssertTrue([result[@"data"] isKindOfClass:[NSDictionary class]]); XCTAssertTrue([result[@"data"][@"type"] isEqualToString:@"Boolean"]); if ([result[@"data"][@"value"] boolValue] == YES) { - [reportExpectation fulfill]; globalReportHandler = nil; + [reportExpectation fulfill]; } }; @@ -1109,13 +1109,34 @@ - (void)test012_SubscriptionError // Wait for report [self waitForExpectations:[NSArray arrayWithObject:reportExpectation] timeout:kTimeoutInSeconds]; - // Trigger reader failure - XCTestExpectation * failureExpectation = [self expectationWithDescription:@"failed on purpose"]; - [device failSubscribers:queue - completion:^{ - [failureExpectation fulfill]; - }]; - [self waitForExpectations:@[ failureExpectation ] timeout:kTimeoutInSeconds]; + XCTestExpectation * errorExpectation1 = [self expectationWithDescription:@"First subscription errored out"]; + XCTestExpectation * errorExpectation2 = [self expectationWithDescription:@"Second subscription errored out"]; + + globalReportHandler = ^(id _Nullable values, NSError * _Nullable error) { + XCTAssertNil(values); + XCTAssertNotNil(error); + globalReportHandler = nil; + [errorExpectation1 fulfill]; + }; + + // Try to create a second subscription, which will cancel the first + // subscription. We can use a non-existent path here to cut down on the + // work that gets done. + params.replaceExistingSubscriptions = YES; + [device subscribeToAttributesWithEndpointID:@10000 + clusterID:@6 + attributeID:@0 + params:params + queue:queue + reportHandler:^(id _Nullable values, NSError * _Nullable error) { + XCTAssertNil(values); + XCTAssertNotNil(error); + [errorExpectation2 fulfill]; + } + subscriptionEstablished:^() { + XCTFail("Did not expect this subscription to succeed"); + }]; + [self waitForExpectations:@[ errorExpectation1, errorExpectation2 ] timeout:60]; deregisterExpectation = [self expectationWithDescription:@"Report handler deregistered"]; [device deregisterReportHandlersWithQueue:queue @@ -1124,7 +1145,7 @@ - (void)test012_SubscriptionError }]; [self waitForExpectations:@[ deregisterExpectation ] timeout:kTimeoutInSeconds]; } -#endif +#endif // DEBUG - (void)test013_ReuseChipClusterObject { diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h index ba0d94e1c341dc..97819f8c99ce07 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h @@ -54,8 +54,6 @@ NS_ASSUME_NONNULL_BEGIN @end @interface MTRBaseDevice (TestDebug) -- (void)failSubscribers:(dispatch_queue_t)queue completion:(void (^)(void))completion; - // Test function for whitebox testing + (id)CHIPEncodeAndDecodeNSObject:(id)object; @end