innoveit
Asynchronous / concurrent GATT operations handling
Describe the bug
react-native-ble-manager does not properly handle GATT operations in a serialized fashion. Although it has a queue for starting operations, it fails to wait for each operation to complete before beginning the next one. Additionally, it does not always call all the pending callbacks with errors when a peripheral is disconnected (at least on Android).
To Reproduce Steps to reproduce the behavior:
- Connect to a BLE peripheral.
- Perform multiple GATT operations (e.g., read, write, notify) in quick succession.
- Disconnect the peripheral during the operations.
Expected behavior
- GATT operations should be executed one at a time, with the next operation starting only after the previous one has completed.
- Callbacks for all pending operations should be called with appropriate errors if the peripheral is disconnected.
Actual Behavior
- GATT operations are not properly serialized, leading to race conditions and potential data corruption.
- Callbacks for pending operations are not always called with errors when the peripheral is disconnected, leading to hanging operations and unresolved promises.
Smartphone (please complete the following information):
Our users that have reported related issues are all using Samsung devices.
Additional context
The current native queue (for both platforms) only starts the GATT operations on serialized fashion, but doesn't wait for the operations to finish before starting the next one. This leads to concurrency issues and potential data corruption. Moreover, the library does not adequately track pending operations, causing callbacks to be missed or not called with appropriate errors when a peripheral is disconnected.
The main culprit for the missed errors from disconnect in Android is this code here:
https://github.com/innoveit/react-native-ble-manager/blob/35004379f0cf4b9615ac8b936d701b7667bddae3/android/src/main/java/it/innove/Peripheral.java#L942-L948
I was able to workaround the concurrency issue by queuing the per peripheral operations on JS. Come to think of it, it makes sense to shift the queuing to javascript, since that code can be shared among the platforms.
Suggested fixes:
- Remove the gatt null check from Android's queue. For more information, see my commit message here:
android: remove gatt null check from nextCommand When a command is queued, it might never be run because queue runner bails out if gatt is null.
Remove this null check so that all the queued commands are run. The callbacks, queues and buffers will be eventually cleared once we hit the disconnected connection state event.
- Improve the serialization either by (a) fixing the native code, (b) add serialization to the javascript code and remove the native queues or (c) at the very least add notes to the README and other documentation that the user knows they need to serialize their GATT operations.
Related issues
- https://github.com/innoveit/react-native-ble-manager/issues/1291
- https://github.com/innoveit/react-native-ble-manager/issues/1211
