[Darwin] Unit test for #39003 (#39041)
* [Darwin] Unit test for #39003
* Update src/darwin/Framework/CHIPTests/MTRDeviceTests.m
Co-authored-by: Kiel Oleson <kielo@apple.com>
---------
Co-authored-by: Justin Wood <woody@apple.com>
Co-authored-by: Kiel Oleson <kielo@apple.com>
diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
index 7f12544..fbf170e 100644
--- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
+++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
@@ -661,6 +661,13 @@
}];
}
+#ifdef DEBUG
+- (void)unitTestSyncRunOnDeviceQueue:(dispatch_block_t)block
+{
+ dispatch_sync(self.queue, block);
+}
+#endif
+
#pragma mark - Time Synchronization
- (void)_setTimeOnDevice
@@ -1072,8 +1079,6 @@
}
os_unfair_lock_unlock(&self->_lock);
- BOOL resubscribeScheduled = NO;
-
if (readClientToResubscribe) {
if (nodeLikelyReachable) {
// If we have reason to suspect the node is now reachable, reset the
@@ -1082,7 +1087,14 @@
// here (e.g. still booting up), but should try again reasonably quickly.
subscriptionCallback->ResetResubscriptionBackoff();
}
- resubscribeScheduled = readClientToResubscribe->TriggerResubscribeIfScheduled(reason.UTF8String);
+
+ BOOL resubscribeScheduled = readClientToResubscribe->TriggerResubscribeIfScheduled(reason.UTF8String);
+
+ // In the case no resubscribe was actually scheduled, remove this device from the subscription pool
+ if (!resubscribeScheduled) {
+ std::lock_guard lock(_lock);
+ [self _clearSubscriptionPoolWork];
+ }
} else if (((_internalDeviceState == MTRInternalDeviceStateSubscribing && !self.doingCASEAttemptForDeviceMayBeReachable) || shouldReattemptSubscription) && nodeLikelyReachable) {
// If we have reason to suspect that the node is now reachable and we haven't established a
// CASE session yet, let's consider it to be stalled and invalidate the pairing session.
@@ -1107,13 +1119,7 @@
// and should be called after the above ReleaseSession call, to avoid churn.
if (shouldReattemptSubscription) {
std::lock_guard lock(_lock);
- resubscribeScheduled = [self _reattemptSubscriptionNowIfNeededWithReason:reason];
- }
-
- // In the case no resubscribe was actually scheduled, remove this device from the subscription pool
- if (!resubscribeScheduled) {
- std::lock_guard lock(_lock);
- [self _clearSubscriptionPoolWork];
+ [self _reattemptSubscriptionNowIfNeededWithReason:reason];
}
}
diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
index a3f9ed8..e1fbb65 100644
--- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -6180,6 +6180,46 @@
delegate.onReachable = nil;
}
+- (void)test048_MTRDeviceResubscribeOnSubscriptionPool
+{
+ __auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId1 deviceController:sController];
+ dispatch_queue_t queue = dispatch_queue_create("subscription-pool-queue", DISPATCH_QUEUE_SERIAL);
+
+ __auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
+ delegate.pretendThreadEnabled = YES;
+ delegate.subscriptionMaxIntervalOverride = @(20); // larger than kTimeoutInSeconds so empty reports do not clear subscription pool sooner
+
+ XCTestExpectation * subscriptionExpectation = [self expectationWithDescription:@"Subscription work completed"];
+ delegate.onReportEnd = ^{
+ [subscriptionExpectation fulfill];
+ };
+
+ [device setDelegate:delegate queue:queue];
+
+ [self waitForExpectations:@[ subscriptionExpectation ] timeout:60];
+
+ // Wait for subscription report stuff to clear and _handleSubscriptionEstablished asynced to device queue
+ [sController syncRunOnWorkQueue:^{
+ ;
+ } error:nil];
+
+ // Wait for _handleSubscriptionEstablished to finish removing subscription work from pool
+ [device unitTestSyncRunOnDeviceQueue:^{
+ ;
+ }];
+
+ // Now we can set up waiting for onSubscriptionPoolWorkComplete from the test
+ XCTestExpectation * subscriptionPoolWorkCompleteForTriggerTestExpectation = [self expectationWithDescription:@"_triggerResubscribeWithReason work completed"];
+ delegate.onSubscriptionPoolWorkComplete = ^{
+ [subscriptionPoolWorkCompleteForTriggerTestExpectation fulfill];
+ };
+
+ // Now that subscription is established and live, ReadClient->mIsResubscriptionScheduled should be false, and _handleResubscriptionNeededWithDelayOnDeviceQueue can simulate the code path that leads to ReadClient->TriggerResubscribeIfScheduled() returning false, and exercise the edge case
+ [device _handleResubscriptionNeededWithDelayOnDeviceQueue:@(0)];
+
+ [self waitForExpectations:@[ subscriptionPoolWorkCompleteForTriggerTestExpectation ] timeout:kTimeoutInSeconds];
+}
+
@end
@interface MTRDeviceEncoderTests : XCTestCase
diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.h
index f1eb607..502e590 100644
--- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.h
+++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.h
@@ -39,6 +39,7 @@
@property (nonatomic, nullable) dispatch_block_t onClusterDataPersisted;
@property (nonatomic, nullable) dispatch_block_t onSubscriptionCallbackDelete;
@property (nonatomic, nullable) dispatch_block_t onSubscriptionReset;
+@property (nonatomic, nullable) NSNumber * subscriptionMaxIntervalOverride;
@end
@interface MTRDeviceTestDelegateWithSubscriptionSetupOverride : MTRDeviceTestDelegate
diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m
index e239ae0..e40bfa5 100644
--- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m
+++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m
@@ -63,6 +63,10 @@
- (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device
{
+ if (self.subscriptionMaxIntervalOverride) {
+ return self.subscriptionMaxIntervalOverride;
+ }
+
// Make sure our subscriptions time out in finite time.
return @(2); // seconds
}
diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h
index 63df5ba..bad3ebb 100644
--- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h
+++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h
@@ -62,6 +62,7 @@
- (NSMutableArray<NSNumber *> *)arrayOfNumbersFromAttributeValue:(MTRDeviceDataValueDictionary)dataDictionary;
- (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;
- (void)_deviceMayBeReachable;
+- (void)_handleResubscriptionNeededWithDelayOnDeviceQueue:(NSNumber *)resubscriptionDelayMs;
@property (nonatomic, readonly, nullable) NSNumber * highestObservedEventNumber;
@property (nonatomic, readonly) MTRAsyncWorkQueue<MTRDevice *> * asyncWorkQueue;
@@ -101,6 +102,7 @@
- (NSSet<MTRClusterPath *> *)unitTestGetPersistedClusters;
- (BOOL)unitTestClusterHasBeenPersisted:(MTRClusterPath *)path;
- (NSUInteger)unitTestAttributeCount;
+- (void)unitTestSyncRunOnDeviceQueue:(dispatch_block_t)block;
@end
#endif
