Avoid capturing this in MTRCommissionableBrowser dispatches (#38158)

Also hold MTRDeviceController reference weakly.

Author: ksperling-apple

src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm

@@ -79,6 +79,10 @@ CHIP_ERROR Start(id<MTRCommissionableBrowserDelegate> delegate, MTRDeviceControl
         VerifyOrReturnError(mDispatchQueue == nil, CHIP_ERROR_INCORRECT_STATE);
         VerifyOrReturnError(mDiscoveredResults == nil, CHIP_ERROR_INCORRECT_STATE);
 
+        VerifyOrReturnError(delegate != nil, CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(controller != nil, CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(queue != nil, CHIP_ERROR_INVALID_ARGUMENT);
+
         mDelegate = delegate;
         mController = controller;
         mDispatchQueue = queue;
@@ -173,6 +177,11 @@ void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
     {
         assertChipStackLockedByCurrentThread();
 
+        // Copy delegate and controller to the stack to avoid capturing `this` in the dispatch_async
+        id<MTRCommissionableBrowserDelegate> delegate = mDelegate;
+        MTRDeviceController * controller = mController;
+        VerifyOrReturn(delegate != nil && controller != nil);
+
         if (!nodeData.Is<CommissionNodeData>()) {
             // not commissionable/commissioners node
             return;
@@ -207,13 +216,10 @@ void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
                 break;
             }
         }
-
-        if (!shouldDispatchToDelegate) {
-            return;
-        }
+        VerifyOrReturn(shouldDispatchToDelegate);
 
         dispatch_async(mDispatchQueue, ^{
-            [mDelegate controller:mController didFindCommissionableDevice:result];
+            [delegate controller:controller didFindCommissionableDevice:result];
         });
     }
 
@@ -245,9 +251,10 @@ void OnBrowseRemove(DnssdService service) override
     {
         assertChipStackLockedByCurrentThread();
 
-        VerifyOrReturn(mDelegate != nil);
-        VerifyOrReturn(mController != nil);
-        VerifyOrReturn(mDispatchQueue != nil);
+        // Copy delegate and controller to the stack to avoid capturing `this` in the dispatch_async
+        id<MTRCommissionableBrowserDelegate> delegate = mDelegate;
+        MTRDeviceController * controller = mController;
+        VerifyOrReturn(delegate != nil && controller != nil);
 
         MATTER_LOG_METRIC(kMetricOnNetworkDevicesRemoved, ++mOnNetworkDevicesRemoved);
 
@@ -278,7 +285,7 @@ void OnBrowseRemove(DnssdService service) override
             // resolving it), so don't bother notifying about the removal either.
             if (result.instanceName != nil) {
                 dispatch_async(mDispatchQueue, ^{
-                    [mDelegate controller:mController didRemoveCommissionableDevice:result];
+                    [delegate controller:controller didRemoveCommissionableDevice:result];
                 });
             }
 
@@ -298,7 +305,11 @@ void OnBrowseStop(CHIP_ERROR error) override
     void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificationInfo & info) override
     {
         assertChipStackLockedByCurrentThread();
-        VerifyOrReturn(mDelegate != nil);
+
+        // Copy delegate and controller to the stack to avoid capturing `this` in the dispatch_async
+        id<MTRCommissionableBrowserDelegate> delegate = mDelegate;
+        MTRDeviceController * controller = mController;
+        VerifyOrReturn(delegate != nil && controller != nil);
 
         auto result = [[MTRCommissionableBrowserResult alloc] init];
         result.instanceName = [NSString stringWithUTF8String:kBleKey];
@@ -315,14 +326,18 @@ void OnBleScanAdd(BLE_CONNECTION_OBJECT connObj, const ChipBLEDeviceIdentificati
         mDiscoveredResults[key] = result;
 
         dispatch_async(mDispatchQueue, ^{
-            [mDelegate controller:mController didFindCommissionableDevice:result];
+            [delegate controller:controller didFindCommissionableDevice:result];
         });
     }
 
     void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
     {
         assertChipStackLockedByCurrentThread();
-        VerifyOrReturn(mDelegate != nil);
+
+        // Copy delegate and controller to the stack to avoid capturing `this` in the dispatch_async
+        id<MTRCommissionableBrowserDelegate> delegate = mDelegate;
+        MTRDeviceController * controller = mController;
+        VerifyOrReturn(delegate != nil && controller != nil);
 
         auto key = [NSString stringWithFormat:@"%@", connObj];
         if ([mDiscoveredResults objectForKey:key] == nil) {
@@ -336,12 +351,13 @@ void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
         MATTER_LOG_METRIC(kMetricBLEDevicesRemoved, ++mBLEDevicesRemoved);
 
         dispatch_async(mDispatchQueue, ^{
-            [mDelegate controller:mController didRemoveCommissionableDevice:result];
+            [delegate controller:controller didRemoveCommissionableDevice:result];
         });
     }
 
     void OnBleScanStopped() override
     {
+        assertChipStackLockedByCurrentThread();
         mBleScannerDelegateOwner = nil;
     }
 
@@ -350,22 +366,21 @@ void OnBleScanStopped() override
 private:
     dispatch_queue_t mDispatchQueue;
     id<MTRCommissionableBrowserDelegate> mDelegate;
-    MTRDeviceController * mController;
+    MTRDeviceController * __weak mController;
     NSMutableDictionary<NSString *, MTRCommissionableBrowserResult *> * mDiscoveredResults;
     int32_t mOnNetworkDevicesAdded;
     int32_t mOnNetworkDevicesRemoved;
     int32_t mBLEDevicesAdded;
     int32_t mBLEDevicesRemoved;
 };
 
-@interface MTRCommissionableBrowser ()
-@property (strong, nonatomic) dispatch_queue_t queue;
-@property (nonatomic, readonly) id<MTRCommissionableBrowserDelegate> delegate;
-@property (nonatomic, readonly) MTRDeviceController * controller;
-@property (unsafe_unretained, nonatomic) CommissionableBrowserInternal browser;
-@end
+@implementation MTRCommissionableBrowser {
+    id<MTRCommissionableBrowserDelegate> _delegate;
+    dispatch_queue_t _queue;
+    MTRDeviceController * __weak _controller;
+    CommissionableBrowserInternal _browser;
+}
 
-@implementation MTRCommissionableBrowser
 - (instancetype)initWithDelegate:(id<MTRCommissionableBrowserDelegate>)delegate
                       controller:(MTRDeviceController *)controller
                            queue:(dispatch_queue_t)queue