From 610b82a542aa6ceb1e74a3740d154e181a2b8dc9 Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Sun, 19 Jul 2026 19:53:12 -0700 Subject: [PATCH 1/3] Make Android write-completion delivery thread-safe Field logs showed writes completing fast (<100ms) or never (timeouts at both 100ms and 1s deadlines, zero GATT-busy errors), sometimes latching into 100% loss until the session ended. The write-completion plumbing can lose acks between onCharacteristicWrite and Dart: 1. writeResultFutureList is an unsynchronized ArrayList mutated from the main thread (writeValue) and Bluetooth binder threads (onCharacteristicWrite / cleanUpConnection) - connectGatt registers no Handler, so callbacks arrive on binder threads. Concurrent add/removeAll can silently drop a pending completion. 2. Pigeon replies were invoked directly on the binder thread; platform channel replies must be sent from the main thread. 3. removeAll invoked completion callbacks inside its predicate: one throwing completion aborts the loop mid-iteration, leaves its entry in the list (double-completed on the next ack), and permanently blocks all later completions - the latched failure mode. Fix: guard every writeResultFutureList access with synchronized; in onCharacteristicWrite and cleanUpConnection, snapshot+remove matches under the lock, then post each completion to the main thread individually wrapped in try/catch. Write-await semantics are unchanged. Co-Authored-By: Claude Fable 5 --- .../universal_ble/UniversalBlePlugin.kt | 86 ++++++++++++------- 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt index d86b75b6..3c417ae8 100644 --- a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt +++ b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt @@ -724,8 +724,12 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), callback ) - // Wait for the result - writeResultFutureList.add(writeFuture) + // Wait for the result. The list is mutated both here (main thread) + // and in onCharacteristicWrite / cleanUpConnection (Bluetooth + // binder threads), so every access must hold the lock. + synchronized(writeResultFutureList) { + writeResultFutureList.add(writeFuture) + } val result = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { gatt.writeCharacteristic(gattCharacteristic, value, writeType) @@ -739,7 +743,9 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), } if (result != BluetoothGatt.GATT_SUCCESS) { - writeResultFutureList.remove(writeFuture) + synchronized(writeResultFutureList) { + writeResultFutureList.remove(writeFuture) + } callback( Result.failure( createFlutterError( @@ -760,30 +766,44 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), characteristic: BluetoothGattCharacteristic, status: Int, ) { - writeResultFutureList.removeAll { - if (it.deviceId == gatt?.device?.address && - it.characteristicId == characteristic.uuid.toString() && - it.serviceId == characteristic.service.uuid.toString() - ) { - if (status == BluetoothGatt.GATT_SUCCESS) { - it.result(Result.success(Unit)) - } else { - UniversalBleLogger.logError( - "WRITE_FAILED <- ${gatt?.device?.address} ${characteristic.uuid} status=$status" - ) - it.result( - Result.failure( - createFlutterError( - gattStatusToUniversalBleErrorCode(status), - "Failed to write", - status.toString() + // Runs on a binder thread: snapshot and remove matches under the lock, + // then complete each future on the main thread (platform-channel + // replies must be sent there). Completing outside the removal loop, + // with each completion individually contained, means one bad callback + // can neither corrupt the list nor block later completions. + val matches: List + synchronized(writeResultFutureList) { + matches = writeResultFutureList.filter { + it.deviceId == gatt?.device?.address && + it.characteristicId == characteristic.uuid.toString() && + it.serviceId == characteristic.service.uuid.toString() + } + writeResultFutureList.removeAll(matches) + } + if (status != BluetoothGatt.GATT_SUCCESS) { + UniversalBleLogger.logError( + "WRITE_FAILED <- ${gatt?.device?.address} ${characteristic.uuid} status=$status" + ) + } + for (future in matches) { + mainThreadHandler?.post { + try { + if (status == BluetoothGatt.GATT_SUCCESS) { + future.result(Result.success(Unit)) + } else { + future.result( + Result.failure( + createFlutterError( + gattStatusToUniversalBleErrorCode(status), + "Failed to write", + status.toString() + ) ) ) - ) + } + } catch (e: Exception) { + UniversalBleLogger.logError("Write completion delivery failed: $e") } - true - } else { - false } } } @@ -1104,12 +1124,18 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), false } } - writeResultFutureList.removeAll { - if (it.deviceId == deviceId) { - it.result(Result.failure(deviceDisconnectedError)) - true - } else { - false + val pendingWrites: List + synchronized(writeResultFutureList) { + pendingWrites = writeResultFutureList.filter { it.deviceId == deviceId } + writeResultFutureList.removeAll(pendingWrites) + } + for (future in pendingWrites) { + mainThreadHandler?.post { + try { + future.result(Result.failure(deviceDisconnectedError)) + } catch (e: Exception) { + UniversalBleLogger.logError("Write completion delivery failed: $e") + } } } subscriptionResultFutureList.removeAll { From 624b25495aef04de24401bbef3b75a29f531a7e5 Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Mon, 20 Jul 2026 21:43:12 -0700 Subject: [PATCH 2/3] Harden write-completion path: @Volatile handler, null-safe service Addresses review feedback on the write-completion thread-safety fix: - mainThreadHandler is read on Bluetooth binder threads but assigned on the main thread; mark it @Volatile so a binder thread cannot observe a stale null and silently drop a completion. - characteristic.service can be null on the binder thread; use a safe call when matching pending futures so an NPE cannot abort the matching loop and leave futures permanently hung. Co-Authored-By: Claude Fable 5 --- .../com/navideck/universal_ble/UniversalBlePlugin.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt index 3c417ae8..a6616903 100644 --- a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt +++ b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt @@ -42,6 +42,12 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), private val advertisePermissionRequestCode = 2342616 private var permissionHandler: PermissionHandler? = null private var callbackChannel: UniversalBleCallbackChannel? = null + + // Read on Bluetooth binder threads (from GATT callbacks) but assigned on the + // main thread in onAttachedToEngine/onDetachedFromEngine; @Volatile makes + // those writes visible so a binder thread never sees a stale null and drops + // a completion. + @Volatile private var mainThreadHandler: Handler? = null private lateinit var context: Context private var activity: Activity? = null @@ -776,7 +782,7 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), matches = writeResultFutureList.filter { it.deviceId == gatt?.device?.address && it.characteristicId == characteristic.uuid.toString() && - it.serviceId == characteristic.service.uuid.toString() + it.serviceId == characteristic.service?.uuid?.toString() } writeResultFutureList.removeAll(matches) } From 5f0cdc34ef8f8a54532cd8a0e6f5b1262a2275ab Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Sun, 26 Jul 2026 12:32:50 -0700 Subject: [PATCH 3/3] Address comment https://github.com/Navideck/universal_ble/pull/268#discussion_r3644457709 --- .../navideck/universal_ble/UniversalBlePlugin.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt index a6616903..f3c767ba 100644 --- a/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt +++ b/android/src/main/kotlin/com/navideck/universal_ble/UniversalBlePlugin.kt @@ -767,6 +767,16 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), } } + // Deliver on the main looper. Platform-channel replies must be sent there, + // and because matching futures are removed from their list before delivery, + // a completion must never be dropped just because the cached handler was + // cleared (a GATT callback racing onDetachedFromEngine) — that would leak + // the pending Dart await. Fall back to a fresh main-looper handler so the + // completion is always attempted. + private fun postToMainLooper(action: () -> Unit) { + (mainThreadHandler ?: Handler(Looper.getMainLooper())).post(action) + } + override fun onCharacteristicWrite( gatt: BluetoothGatt?, characteristic: BluetoothGattCharacteristic, @@ -792,7 +802,7 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), ) } for (future in matches) { - mainThreadHandler?.post { + postToMainLooper { try { if (status == BluetoothGatt.GATT_SUCCESS) { future.result(Result.success(Unit)) @@ -1136,7 +1146,7 @@ class UniversalBlePlugin : UniversalBlePlatformChannel, BluetoothGattCallback(), writeResultFutureList.removeAll(pendingWrites) } for (future in pendingWrites) { - mainThreadHandler?.post { + postToMainLooper { try { future.result(Result.failure(deviceDisconnectedError)) } catch (e: Exception) {