fix(ble): cleanup races discovered while reviewing #5207 (#5221)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: garthvh <1795163+garthvh@users.noreply.github.com>

Author: 3 people

core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleConnection.kt

@@ -139,10 +139,17 @@ class KableBleConnection(private val scope: CoroutineScope) : BleConnection {
             meshtasticDevice.advertisement?.let { adv -> Peripheral(adv) { commonConfig() } }
                 ?: createPeripheral(device.address) { commonConfig() }
 
-        cleanUpPeripheral(device.address)
-        peripheral = p
-
-        ActiveBleConnection.active = ActiveConnection(p, device.address)
+        // Install ownership of the new peripheral atomically. Cancellation between
+        // peripheral construction and field assignment would strand `p` (Kable allocates
+        // a per-peripheral scope + Bluetooth-state observer eagerly), so the cleanup,
+        // assignment, and ActiveBleConnection update must complete as a single unit.
+        // _deviceFlow.emit() is intentionally outside this block — making it
+        // non-cancellable could hang teardown on a slow collector.
+        withContext(NonCancellable) {
+            cleanUpPeripheral(device.address)
+            peripheral = p
+            ActiveBleConnection.active = ActiveConnection(p, device.address)
+        }
 
         _deviceFlow.emit(device)
 
@@ -212,11 +219,18 @@ class KableBleConnection(private val scope: CoroutineScope) : BleConnection {
         stateJob?.cancel()
         stateJob = null
 
+        // Capture the peripheral we own before clearing it so we can identity-check
+        // ActiveBleConnection below. A stale disconnect from an earlier connection
+        // attempt's exception handler must not clobber a newer connection that has
+        // already installed itself as active.
+        val owned = peripheral
         safeClosePeripheral("disconnect")
         peripheral = null
         connectionScope = null
 
-        ActiveBleConnection.active = null
+        if (owned != null && ActiveBleConnection.active?.peripheral === owned) {
+            ActiveBleConnection.active = null
+        }
 
         _deviceFlow.emit(null)
     }

core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt

@@ -103,9 +103,17 @@ class BleRadioTransport(
     internal val address: String,
 ) : RadioTransport {
 
+    // Detached cleanup scope for last-ditch GATT teardown from the exception handler.
+    // Must NOT be a child of `scope`: when an uncaught exception fires in connectionScope,
+    // upper layers often tear down `scope` immediately. Launching cleanup on `scope` then
+    // races the cancellation and may never start, leaking BluetoothGatt (status 133 on
+    // the next reconnect). This scope is cancelled in close() once our own disconnect
+    // has completed and the safety net is no longer needed.
+    private val cleanupScope: CoroutineScope = CoroutineScope(SupervisorJob() + scope.coroutineContext.minusKey(Job))
+
     private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
         Logger.w(throwable) { "[$address] Uncaught exception in connectionScope" }
-        scope.launch {
+        cleanupScope.launch {
             try {
                 bleConnection.disconnect()
             } catch (e: Exception) {
@@ -427,6 +435,10 @@ class BleRadioTransport(
                 Logger.w(e) { "[$address] Failed to disconnect in close()" }
             }
         }
+        // Our own disconnect succeeded — the exception-handler safety net is no longer
+        // needed. Cancel the detached cleanup scope so it doesn't outlive us in tests
+        // or process lifetime.
+        cleanupScope.cancel("close() called")
     }
 
     private fun dispatchPacket(packet: ByteArray) {