Darwin: Bug fixes in BleConnectionDelegateImpl

* Call HandleConnectionError instead of CloseAllBleConnections when
  force-closing a connection because we're discarding the BleConnection holder.
* Capture the CBPeripheral strong ref instead of the void * in dispatches.
* Also read characteristic value before dispatching and add some logging.

src/platform/Darwin/BleConnectionDelegateImpl.mm

@@ -140,6 +140,7 @@ - (void)removePeripheralsFromCache;
                 bleWorkQueue = dispatch_queue_create(kBleWorkQueueName, DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
             }
 
+            CBPeripheral * peripheral = (__bridge CBPeripheral *) connObj; // bridge (and retain) before dispatching
             dispatch_async(bleWorkQueue, ^{
                 // The BLE_CONNECTION_OBJECT represent a CBPeripheral object. In order for it to be valid the central
                 // manager needs to still be running.
@@ -157,7 +158,7 @@ - (void)removePeripheralsFromCache;
                 ble.appState = appState;
                 ble.onConnectionComplete = OnConnectionComplete;
                 ble.onConnectionError = OnConnectionError;
-                [ble updateWithPeripheral:(__bridge CBPeripheral *) connObj];
+                [ble updateWithPeripheral:peripheral];
             });
         }
 
@@ -285,7 +286,6 @@ - (void)setupTimer:(uint64_t)timeout
     _timer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, _workQueue);
     dispatch_source_set_event_handler(_timer, ^{
         [self stop];
-        [self dispatchConnectionError:BLE_ERROR_APP_CLOSED_CONNECTION];
     });
 
     auto value = static_cast<int64_t>(timeout * NSEC_PER_SEC);
@@ -304,19 +304,31 @@ - (void)clearTimer
 // All our callback dispatch must happen on _chipWorkQueue
 - (void)dispatchConnectionError:(CHIP_ERROR)error
 {
+    dispatch_assert_queue_debug(_workQueue);
+    auto onConnectionError = self.onConnectionError;
+    VerifyOrReturn(onConnectionError != nullptr);
+
+    auto appState = self.appState;
+    self.appState = nullptr;
+    self.onConnectionError = nullptr;
+    self.onConnectionComplete = nullptr;
     dispatch_async(_chipWorkQueue, ^{
-        if (self.onConnectionError != nil) {
-            self.onConnectionError(self.appState, error);
-        }
+        onConnectionError(appState, error);
     });
 }
 
 - (void)dispatchConnectionComplete:(CBPeripheral *)peripheral
 {
+    dispatch_assert_queue_debug(_workQueue);
+    auto onConnectionComplete = self.onConnectionComplete;
+    VerifyOrReturn(onConnectionComplete != nullptr);
+
+    auto appState = self.appState;
+    self.appState = nullptr;
+    self.onConnectionError = nullptr;
+    self.onConnectionComplete = nullptr;
     dispatch_async(_chipWorkQueue, ^{
-        if (self.onConnectionComplete != nil) {
-            self.onConnectionComplete(self.appState, (__bridge void *) peripheral);
-        }
+        onConnectionComplete(appState, (__bridge void *) peripheral);
     });
 }
 
@@ -332,7 +344,6 @@ - (void)centralManagerDidUpdateState:(CBCentralManager *)central
     case CBManagerStatePoweredOff:
         ChipLogDetail(Ble, "CBManagerState: OFF");
         [self stop];
-        [self dispatchConnectionError:BLE_ERROR_APP_CLOSED_CONNECTION];
         break;
     case CBManagerStateUnauthorized:
         ChipLogDetail(Ble, "CBManagerState: Unauthorized");
@@ -521,10 +532,11 @@ - (void)peripheral:(CBPeripheral *)peripheral
         chip::Ble::ChipBleUUID svcId;
         chip::Ble::ChipBleUUID charId;
         [BleConnection fillServiceWithCharacteristicUuids:characteristic svcId:&svcId charId:&charId];
+        auto * value = characteristic.value; // read immediately before dispatching
 
         dispatch_async(_chipWorkQueue, ^{
             // build a inet buffer from the rxEv and send to blelayer.
-            auto msgBuf = chip::System::PacketBufferHandle::NewWithData(characteristic.value.bytes, characteristic.value.length);
+            auto msgBuf = chip::System::PacketBufferHandle::NewWithData(value.bytes, value.length);
 
             if (msgBuf.IsNull()) {
                 ChipLogError(Ble, "Failed at allocating buffer for incoming BLE data");
@@ -559,33 +571,29 @@ - (void)start
 
 - (void)stop
 {
+    dispatch_assert_queue_debug(_workQueue);
     _scannerDelegate = nil;
     _found = false;
     [self stopScanning];
     [self removePeripheralsFromCache];
 
-    if (!_centralManager && !_peripheral) {
-        return;
+    // Call connection error completion in case we were still in the process of connecting
+    [self dispatchConnectionError:BLE_ERROR_APP_CLOSED_CONNECTION];
+
+    // Signal a connection error in case the peripheral was adopted by a BLEEndPoint
+    auto peripheral = _peripheral;
+    if (peripheral) {
+        ChipLogProgress(Ble, "Preparing to drop peripheral %p", peripheral);
+        dispatch_async(_chipWorkQueue, ^{
+            _mBleLayer->HandleConnectionError((__bridge void *) peripheral, BLE_ERROR_APP_CLOSED_CONNECTION);
+        });
+        _peripheral = nil;
     }
 
-    // Properly closing the underlying ble connections needs to happens
-    // on the chip work queue. At the same time the SDK is trying to
-    // properly unsubscribe and shutdown the connection, so if we nullify
-    // the centralManager and the peripheral members too early it won't be
-    // able to reach those.
-    // This is why closing connections happens as 2 async steps.
-    dispatch_async(_chipWorkQueue, ^{
-        if (_peripheral) {
-            _mBleLayer->CloseAllBleConnections();
-        }
+    _centralManager.delegate = nil;
+    _centralManager = nil;
 
-        dispatch_async(_workQueue, ^{
-            _centralManager.delegate = nil;
-            _centralManager = nil;
-            _peripheral = nil;
-            chip::DeviceLayer::Internal::ble = nil;
-        });
-    });
+    chip::DeviceLayer::Internal::ble = nil; // unreference self (this is not great)
 }
 
 - (void)startScanning