From 79ef679768b264679240ecbdcebb46e45f4dd319 Mon Sep 17 00:00:00 2001
From: Vivien Nicolas <vnicolas@apple.com>
Date: Tue, 7 Mar 2023 13:40:00 +0100
Subject: [PATCH] [darwin] Race when BleConnection stop is called

---
 .../Darwin/BleConnectionDelegateImpl.mm       | 98 ++++++++++++-------
 1 file changed, 62 insertions(+), 36 deletions(-)

diff --git a/src/platform/Darwin/BleConnectionDelegateImpl.mm b/src/platform/Darwin/BleConnectionDelegateImpl.mm
index 004a33bfb0e963..3468fee7e9d598 100644
--- a/src/platform/Darwin/BleConnectionDelegateImpl.mm
+++ b/src/platform/Darwin/BleConnectionDelegateImpl.mm
@@ -32,6 +32,7 @@
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/CHIPDeviceLayer.h>
 #include <platform/Darwin/BleConnectionDelegate.h>
+#include <platform/LockTracker.h>
 #include <setup_payload/SetupPayload.h>
 
 #import "UUIDHelper.h"
@@ -40,10 +41,10 @@
 
 constexpr uint64_t kScanningWithDiscriminatorTimeoutInSeconds = 60;
 constexpr uint64_t kScanningWithoutDiscriminatorTimeoutInSeconds = 120;
+constexpr const char * kBleWorkQueueName = "org.csa-iot.matter.framework.ble.workqueue";
 
 @interface BleConnection : NSObject <CBCentralManagerDelegate, CBPeripheralDelegate>
 
-@property (strong, nonatomic) dispatch_queue_t workQueue;
 @property (strong, nonatomic) dispatch_queue_t chipWorkQueue;
 @property (strong, nonatomic) CBCentralManager * centralManager;
 @property (strong, nonatomic) CBPeripheral * peripheral;
@@ -58,7 +59,8 @@ @interface BleConnection : NSObject <CBCentralManagerDelegate, CBPeripheralDeleg
 @property (unsafe_unretained, nonatomic) BleConnectionDelegate::OnConnectionErrorFunct onConnectionError;
 @property (unsafe_unretained, nonatomic) chip::Ble::BleLayer * mBleLayer;
 
-- (id)initWithDiscriminator:(const chip::SetupDiscriminator &)deviceDiscriminator;
+- (id)initWithQueue:(dispatch_queue_t)queue;
+- (id)initWithDiscriminator:(const chip::SetupDiscriminator &)deviceDiscriminator queue:(dispatch_queue_t)queue;
 - (void)setBleLayer:(chip::Ble::BleLayer *)bleLayer;
 - (void)start;
 - (void)stop;
@@ -72,57 +74,80 @@ - (void)update;
 namespace DeviceLayer {
     namespace Internal {
         BleConnection * ble;
+        dispatch_queue_t bleWorkQueue;
 
         void BleConnectionDelegateImpl::NewConnection(
             Ble::BleLayer * bleLayer, void * appState, const SetupDiscriminator & deviceDiscriminator)
         {
+            assertChipStackLockedByCurrentThread();
+
             ChipLogProgress(Ble, "%s", __FUNCTION__);
+            if (!bleWorkQueue) {
+                bleWorkQueue = dispatch_queue_create(kBleWorkQueueName, DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
+            }
+
+            dispatch_async(bleWorkQueue, ^{
+                // If the previous connection delegate was a scan without a discriminator, just reuse it instead of
+                // creating a brand new connection but update the discriminator and the ble layer members.
+                if (ble and ![ble hasDiscriminator]) {
+                    [ble setBleLayer:bleLayer];
+                    ble.appState = appState;
+                    ble.onConnectionComplete = OnConnectionComplete;
+                    ble.onConnectionError = OnConnectionError;
+                    [ble updateWithDiscriminator:deviceDiscriminator];
+                    return;
+                }
 
-            // If the previous connection delegate was a scan without a discriminator, just reuse it instead of
-            // creating a brand new connection but update the discriminator and the ble layer members.
-            if (ble and ![ble hasDiscriminator]) {
+                [ble stop];
+                ble = [[BleConnection alloc] initWithDiscriminator:deviceDiscriminator queue:bleWorkQueue];
                 [ble setBleLayer:bleLayer];
                 ble.appState = appState;
                 ble.onConnectionComplete = OnConnectionComplete;
                 ble.onConnectionError = OnConnectionError;
-                [ble updateWithDiscriminator:deviceDiscriminator];
-                return;
-            }
-
-            CancelConnection();
-            ble = [[BleConnection alloc] initWithDiscriminator:deviceDiscriminator];
-            [ble setBleLayer:bleLayer];
-            ble.appState = appState;
-            ble.onConnectionComplete = OnConnectionComplete;
-            ble.onConnectionError = OnConnectionError;
-            ble.centralManager = [ble.centralManager initWithDelegate:ble queue:ble.workQueue];
+                ble.centralManager = [ble.centralManager initWithDelegate:ble queue:bleWorkQueue];
+            });
         }
 
         void BleConnectionDelegateImpl::PrepareConnection()
         {
-            ChipLogProgress(Ble, "%s", __FUNCTION__);
+            assertChipStackLockedByCurrentThread();
 
-            // If the previous connection delegate was a scan without a discriminator, just reuse it instead of
-            // creating a brand new connection but clear the cache and reset the timer.
-            if (ble and ![ble hasDiscriminator]) {
-                [ble update];
-                return;
+            ChipLogProgress(Ble, "%s", __FUNCTION__);
+            if (!bleWorkQueue) {
+                bleWorkQueue = dispatch_queue_create(kBleWorkQueueName, DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
             }
 
-            CancelConnection();
-            ble = [[BleConnection alloc] init];
-            ble.onConnectionComplete = OnConnectionComplete;
-            ble.onConnectionError = OnConnectionError;
-            ble.centralManager = [ble.centralManager initWithDelegate:ble queue:ble.workQueue];
+            dispatch_async(bleWorkQueue, ^{
+                // If the previous connection delegate was a scan without a discriminator, just reuse it instead of
+                // creating a brand new connection but clear the cache and reset the timer.
+                if (ble and ![ble hasDiscriminator]) {
+                    [ble update];
+                    return;
+                }
+
+                [ble stop];
+                ble = [[BleConnection alloc] initWithQueue:bleWorkQueue];
+                ble.onConnectionComplete = OnConnectionComplete;
+                ble.onConnectionError = OnConnectionError;
+                ble.centralManager = [ble.centralManager initWithDelegate:ble queue:bleWorkQueue];
+            });
         }
 
         CHIP_ERROR BleConnectionDelegateImpl::CancelConnection()
         {
+            assertChipStackLockedByCurrentThread();
+
             ChipLogProgress(Ble, "%s", __FUNCTION__);
-            if (ble) {
+            if (bleWorkQueue == nil) {
+                return CHIP_NO_ERROR;
+            }
+
+            dispatch_async(bleWorkQueue, ^{
                 [ble stop];
                 ble = nil;
-            }
+            });
+
+            bleWorkQueue = nil;
             return CHIP_NO_ERROR;
         }
     } // namespace Internal
@@ -134,15 +159,13 @@ @interface BleConnection ()
 
 @implementation BleConnection
 
-- (id)init
+- (id)initWithQueue:(dispatch_queue_t)queue
 {
     self = [super init];
     if (self) {
         self.shortServiceUUID = [UUIDHelper GetShortestServiceUUID:&chip::Ble::CHIP_BLE_SVC_ID];
-        _workQueue
-            = dispatch_queue_create("org.csa-iot.matter.framework.ble.workqueue", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
         _chipWorkQueue = chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue();
-        _timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, _workQueue);
+        _timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, queue);
         _centralManager = [CBCentralManager alloc];
         _found = false;
         _cachedPeripherals = [[NSMutableDictionary alloc] init];
@@ -159,9 +182,9 @@ - (id)init
     return self;
 }
 
-- (id)initWithDiscriminator:(const chip::SetupDiscriminator &)deviceDiscriminator
+- (id)initWithDiscriminator:(const chip::SetupDiscriminator &)deviceDiscriminator queue:(dispatch_queue_t)queue
 {
-    self = [self init];
+    self = [self initWithQueue:queue];
     if (self) {
         _deviceDiscriminator = deviceDiscriminator;
         _hasDeviceDiscriminator = true;
@@ -460,7 +483,10 @@ - (void)disconnect
         return;
     }
 
-    _mBleLayer->CloseAllBleConnections();
+    dispatch_sync(_chipWorkQueue, ^{
+        _mBleLayer->CloseAllBleConnections();
+    });
+
     _peripheral = nil;
 }