project-chip · woody-apple · May 17, 2025 · May 17, 2025 · May 17, 2025
@@ -660,6 +660,13 @@ - (void)_notifyDelegateOfPrivateInternalPropertiesChanges
     }];
 }
 
+#ifdef DEBUG
+- (void)unitTestSyncRunOnDeviceQueue:(dispatch_block_t)block
+{
+    dispatch_sync(self.queue, block);
+}
+#endif
+
 #pragma mark - Time Synchronization
 
 - (void)_setTimeOnDevice
@@ -1071,8 +1078,6 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
     }
     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
@@ -1081,7 +1086,14 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
             // 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.
@@ -1106,13 +1118,7 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
     // 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];
     }
 }
 

@@ -6180,6 +6180,46 @@ - (void)test047_DeviceMaybeReachableSetsUnreachable
     delegate.>
 }
 
+- (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.>
+        [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.>
+        [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

@@ -39,6 +39,7 @@ typedef void (^MTRDeviceTestDelegateDataHandler)(NSArray<NSDictionary<NSString *
 @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

@@ -63,6 +63,10 @@ - (void)unitTestReportEndForDevice:(MTRDevice *)device
 
 - (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device
 {
+    if (self.subscriptionMaxIntervalOverride) {
+        return self.subscriptionMaxIntervalOverride;
+    }
+
     // Make sure our subscriptions time out in finite time.
     return @(2); // seconds
 }

@@ -62,6 +62,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (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 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSSet<MTRClusterPath *> *)unitTestGetPersistedClusters;
 - (BOOL)unitTestClusterHasBeenPersisted:(MTRClusterPath *)path;
 - (NSUInteger)unitTestAttributeCount;
+- (void)unitTestSyncRunOnDeviceQu
5150
eue:(dispatch_block_t)block;
 @end
 #endif