[Darwin] MTRDeviceController to limit concurrent subscriptions to Thread-enabled devices

.github/workflows/darwin.yaml

@@ -115,6 +115,11 @@ jobs:
                   echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
                   ../../../out/debug/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/darwin/framework-tests/end_user_support_log.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
                   ../../../out/debug/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) &
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 101 --passcode 1001 --KVS /tmp/chip-all-clusters-app-kvs101 --secured-device-port 5531 &
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 102 --passcode 1002 --KVS /tmp/chip-all-clusters-app-kvs102 --secured-device-port 5532 &
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 103 --passcode 1003 --KVS /tmp/chip-all-clusters-app-kvs103 --secured-device-port 5533 &
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 104 --passcode 1004 --KVS /tmp/chip-all-clusters-app-kvs104 --secured-device-port 5534 &
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --discriminator 105 --passcode 1005 --KVS /tmp/chip-all-clusters-app-kvs105 --secured-device-port 5535 &
 
                   export TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1
 

src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.h

@@ -145,8 +145,6 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
     NSMutableArray * _Nullable mEventReports = nil;
 
     void CallResubscriptionScheduledHandler(NSError * error, NSNumber * resubscriptionDelay);
-    // Copied from ReadClient and customized for MTRDevice resubscription time reset
-    uint32_t ComputeTimeTillNextSubscription();
 
 private:
     DataReportCallback _Nullable mAttributeReportCallback = nil;

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -362,6 +362,9 @@ - (BOOL)unitTestShouldSetUpSubscriptionForDevice:(MTRDevice *)device;
 - (BOOL)unitTestShouldSkipExpectedValuesForWrite:(MTRDevice *)device;
 - (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device;
 - (BOOL)unitTestForceAttributeReportsIfMatchingCache:(MTRDevice *)device;
+- (BOOL)unitTestPretendThreadEnabled:(MTRDevice *)device;
+- (void)unitTestSubscriptionPoolDequeue:(MTRDevice *)device;
+- (void)unitTestSubscriptionPoolWorkComplete:(MTRDevice *)device;
 @end
 #endif
 
@@ -393,10 +396,10 @@ @implementation MTRDevice {
     // If this is true when the report ends, we notify the delegate.
     BOOL _deviceConfigurationChanged;
 
-    // The completion block is set when the subscription / resubscription work is enqueued, and called / cleared when:
+    // The completion block is set when the subscription / resubscription work is enqueued, and called / cleared when any of the following happen:
     //   1. Subscription establishes
-    //   2. When OnResubscriptionNeeded is called
-    //   3. On subscription reset (including when getSessionForNode fails)
+    //   2. OnResubscriptionNeeded is called
+    //   3. Subscription reset (including when getSessionForNode fails)
     MTRAsyncWorkCompletionBlock _subscriptionPoolWorkCompletionBlock;
 }
 
@@ -689,7 +692,14 @@ - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queu
     }
 
     if (setUpSubscription) {
-        [self _setupSubscription];
+        if ([self _deviceUsesThread]) {
+            [self _scheduleSubscriptionPoolWork:^{
+                std::lock_guard lock(self->_lock);
+                [self _setupSubscription];
+            } inNanoseconds:0 description:@"MTRDevice setDelegate first subscription"];
+        } else {
+            [self _setupSubscription];
+        }
     }
 }
 
@@ -940,43 +950,66 @@ - (void)_handleSubscriptionError:(NSError *)error
 
 // This method is used for signaling whether to use the subscription pool. This functions as
 // a heuristic for whether to throttle subscriptions to the device via a pool of subscriptions.
-// If products appear that have both Thread and Wifi enabled but is primarily on wifi, this
+// If products appear that have both Thread and Wifi enabled but are primarily on wifi, this
 // method will need to be updated to reflect that.
 - (BOOL)_deviceUsesThread
 {
     os_unfair_lock_assert_owner(&self->_lock);
 
-    // Device is thread-enabled if there is a Thread Network Diagnostics cluster on endpoint 0
-    for (MTRClusterPath * path in [self _knownClusters]) {
-        if (path.endpoint.unsignedShortValue != 0) {
-            continue;
+#ifdef DEBUG
+    id testDelegate = _weakDelegate.strongObject;
+    if (testDelegate) {
+        if ([testDelegate respondsToSelector:@selector(unitTestPretendThreadEnabled:)]) {
+            if ([testDelegate unitTestPretendThreadEnabled:self]) {
+                return YES;
+            }
         }
+    }
+#endif
 
-        if (path.cluster.unsignedLongValue == MTRClusterIDTypeThreadNetworkDiagnosticsID) {
-            return YES;
-        }
+    MTRClusterPath * networkCommissioningClusterPath = [MTRClusterPath clusterPathWithEndpointID:@(0) clusterID:@(MTRClusterIDTypeNetworkCommissioningID)];
+    MTRDeviceClusterData * networkCommissioningClusterData = [self _clusterDataForPath:networkCommissioningClusterPath];
+    NSNumber * networkCommissioningClusterFeatureMapValueNumber = networkCommissioningClusterData.attributes[@(MTRClusterGlobalAttributeFeatureMapID)][MTRValueKey];
+    if (![networkCommissioningClusterFeatureMapValueNumber isKindOfClass:[NSNumber class]]) {
+        MTR_LOG_ERROR("%@ Unexpected NetworkCommissioning FeatureMap value %@", self, networkCommissioningClusterFeatureMapValueNumber);
+        return NO;
     }
 
-    return NO;
+    // Bit 0 WiFi
+    // Bit 1 Thread
+    // Bit 2 Ethernet
+    uint32_t networkCommissioningClusterFeatureMapValue = static_cast<uint32_t>(networkCommissioningClusterFeatureMapValueNumber.unsignedLongValue);
+
+    return (networkCommissioningClusterFeatureMapValue & (1 << 1)) ? YES : NO;
 }
 
 - (void)_clearSubscriptionPoolWork
 {
     os_unfair_lock_assert_owner(&self->_lock);
     MTRAsyncWorkCompletionBlock completion = self->_subscriptionPoolWorkCompletionBlock;
     if (completion) {
+#ifdef DEBUG
+        id delegate = self->_weakDelegate.strongObject;
+        if (delegate) {
+            dispatch_async(self->_delegateQueue, ^{
+                if ([delegate respondsToSelector:@selector(unitTestSubscriptionPoolWorkComplete:)]) {
+                    [delegate unitTestSubscriptionPoolWorkComplete:self];
+                }
+            });
+        }
+#endif
         self->_subscriptionPoolWorkCompletionBlock = nil;
         completion(MTRAsyncWorkComplete);
     }
 }
 
-- (void)_scheduleSubscriptionPoolWork:(void (^)(void))workBlock inNanoseconds:(int64_t)inNanoseconds description:(NSString *)description
+- (void)_scheduleSubscriptionPoolWork:(dispatch_block_t)workBlock inNanoseconds:(int64_t)inNanoseconds description:(NSString *)description
 {
     os_unfair_lock_assert_owner(&self->_lock);
 
     // Sanity check we are not scheduling for this device multiple times in the pool
     if (_subscriptionPoolWorkCompletionBlock) {
-        MTR_LOG_DEFAULT("%@ already scheduled in subscription pool for this device - ignoring: %@", self, description);
+        MTR_LOG_ERROR("%@ already scheduled in subscription pool for this device - ignoring: %@", self, description);
         return;
     }
 
@@ -986,6 +1019,16 @@ - (void)_scheduleSubscriptionPoolWork:(void (^)(void))workBlock inNanoseconds:(i
         MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:self.queue];
         [workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull completion) {
             os_unfair_lock_lock(&self->_lock);
+#ifdef DEBUG
+            id delegate = self->_weakDelegate.strongObject;
+            if (delegate) {
+                dispatch_async(self->_delegateQueue, ^{
+                    if ([delegate respondsToSelector:@selector(unitTestSubscriptionPoolDequeue:)]) {
+                        [delegate unitTestSubscriptionPoolDequeue:self];
+                    }
+                });
+            }
+#endif
             if (self->_subscriptionPoolWorkCompletionBlock) {
                 // This means a resubscription triggering event happened and is now in-progress
                 MTR_LOG_DEFAULT("%@ timer fired but already running in subscription pool - ignoring: %@", self, description);

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -145,7 +145,8 @@ - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParamete
     return [MTRDeviceControllerFactory.sharedInstance initializeController:self withParameters:controllerParameters error:error];
 }
 
-constexpr NSUInteger kDefaultConcurrentSubscriptionPoolSize = 3;
+static NSString * const kLocalTestUserDefaultDomain = @"org.csa-iot.matter.darwintest";
+static NSString * const kLocalTestUserDefaultSubscriptionPoolSizeOverrideKey = @"subscriptionPoolSizeOverride";
 
 - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
                              queue:(dispatch_queue_t)queue
@@ -254,8 +255,19 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
             return nil;
         }
 
+        // Provide a way to test different subscription pool sizes without code change
+        NSUserDefaults * defaults = [[NSUserDefaults alloc] initWithSuiteName:kLocalTestUserDefaultDomain];
+        if ([defaults objectForKey:kLocalTestUserDefaultSubscriptionPoolSizeOverrideKey]) {
+            NSInteger subscriptionPoolSizeOverride = [defaults integerForKey:kLocalTestUserDefaultSubscriptionPoolSizeOverrideKey];
+            if (subscriptionPoolSizeOverride < 1) {
+                concurrentSubscriptionPoolSize = 1;
+            } else {
+                concurrentSubscriptionPoolSize = static_cast<NSUInteger>(subscriptionPoolSizeOverride);
+            }
+        }
+
         if (!concurrentSubscriptionPoolSize) {
-            concurrentSubscriptionPoolSize = kDefaultConcurrentSubscriptionPoolSize;
+            concurrentSubscriptionPoolSize = 1;
         }
         _concurrentSubscriptionPool = [[MTRAsyncWorkQueue alloc] initWithContext:self width:concurrentSubscriptionPoolSize];
 

src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h

@@ -79,8 +79,10 @@ MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6))
 
 /**
  * Sets the maximum subscriptions allowed for devices on Thread. This defaults to 3.
+ *
+ * If this value is 0, the maximum subscriptinos allowed will be set to 1.
  */
-@property (nonatomic, assign) NSUInteger concurrentSubscriptionsAllowedOnThread;
+@property (nonatomic, assign) NSUInteger concurrentSubscriptionsAllowedOnThread MTR_NEWLY_AVAILABLE;
 
 @end
 

src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.mm

@@ -254,6 +254,8 @@ - (instancetype)_initInternal
 }
 @end
 
+constexpr NSUInteger kDefaultConcurrentSubscriptionPoolSize = 300;
+
 @implementation MTRDeviceControllerParameters
 - (instancetype)initWithStorageDelegate:(id<MTRDeviceControllerStorageDelegate>)storageDelegate
                    storageDelegateQueue:(dispatch_queue_t)storageDelegateQueue
@@ -286,6 +288,8 @@ - (instancetype)initWithStorageDelegate:(id<MTRDeviceControllerStorageDelegate>)
     _storageDelegateQueue = storageDelegateQueue;
     _uniqueIdentifier = uniqueIdentifier;
 
+    _concurrentSubscriptionsAllowedOnThread = kDefaultConcurrentSubscriptionPoolSize;
+
     return self;
 }
 

src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m

@@ -34,8 +34,13 @@
 static const uint16_t kTestProductId2 = 0x8001u;
 static const uint16_t kTestDiscriminator1 = 3840u;
 static const uint16_t kTestDiscriminator2 = 3839u;
+static const uint16_t kTestDiscriminator3 = 101u;
+static const uint16_t kTestDiscriminator4 = 102u;
+static const uint16_t kTestDiscriminator5 = 103u;
+static const uint16_t kTestDiscriminator6 = 104u;
+static const uint16_t kTestDiscriminator7 = 105u;
 static const uint16_t kDiscoverDeviceTimeoutInSeconds = 10;
-static const uint16_t kExpectedDiscoveredDevicesCount = 2;
+static const uint16_t kExpectedDiscoveredDevicesCount = 7;
 
 // Singleton controller we use.
 static MTRDeviceController * sController = nil;
@@ -97,7 +102,7 @@ - (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice
     XCTAssertEqual(instanceName.length, 16); // The  instance name is random, so just ensure the len is right.
     XCTAssertEqualObjects(vendorId, @(kTestVendorId));
     XCTAssertTrue([productId isEqual:@(kTestProductId1)] || [productId isEqual:@(kTestProductId2)]);
-    XCTAssertTrue([discriminator isEqual:@(kTestDiscriminator1)] || [discriminator isEqual:@(kTestDiscriminator2)]);
+    XCTAssertTrue([discriminator isEqual:@(kTestDiscriminator1)] || [discriminator isEqual:@(kTestDiscriminator2)] || [discriminator isEqual:@(kTestDiscriminator3)] || [discriminator isEqual:@(kTestDiscriminator4)] || [discriminator isEqual:@(kTestDiscriminator5)] || [discriminator isEqual:@(kTestDiscriminator6)] || [discriminator isEqual:@(kTestDiscriminator7)]);
     XCTAssertEqual(commissioningMode, YES);
 
     NSLog(@"Found Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);