Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous / concurrent GATT operations handling #1333

Open
vhakulinen opened this issue Feb 7, 2025 · 0 comments
Open

Asynchronous / concurrent GATT operations handling #1333

vhakulinen opened this issue Feb 7, 2025 · 0 comments

Comments

@vhakulinen
Copy link
Contributor

vhakulinen commented Feb 7, 2025

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:

  1. Connect to a BLE peripheral.
  2. Perform multiple GATT operations (e.g., read, write, notify) in quick succession.
  3. 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:

// Check if we still have a valid gatt object
if (gatt == null) {
Log.d(BleManager.LOG_TAG, "Error, gatt is null. Fill all callbacks with an error");
errorAndClearAllCallbacks("Gatt is null");
resetQueuesAndBuffers();
return;
}

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant