New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 647673 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 490399


Show other hotlists

Hotlists containing this issue:
web-bluetooth


Sign in to add a comment

Possible race conditions in Android GATT notifications

Reported by emil.len...@gmail.com, Sep 16 2016

Issue description

Look at https://chromium.googlesource.com/chromium/src/+/d5980418932d12bfa1f6884473247961d1e4151b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java#197

Once a notification is arrived, work is posted to another thread which then reads the latest characteristic value. If multiple notifications arrive fast, this could result in race conditions where all the onCharacteristicChanged callbacks are first triggered, then the three tasks posted are executed which all extract the latest notified value instead of the individual values. The value should be extracted at the beginning of the onCharacteristicChanged callback rather than in the posted task to minimize risk for race condition.

In Android's GATT implementation, the notified value is assigned to the characteristic just before the callback is fired (https://android.googlesource.com/platform/frameworks/base/+/8e970d6ab4850ca23163bad5773cf5cc2e7ff896/core/java/android/bluetooth/BluetoothGatt.java#348).

From my experimentation, it seems the GATT callbacks are serialized (a new callback is never fired until the old one has returned) so reading the value in the callback should remove the race condition. However there's still a race condition if there is a write at the same time as a notification arrives (see http://stackoverflow.com/questions/38922639/how-could-i-achieve-maximum-thread-safety-with-a-read-write-ble-gatt-characteris).

---

 issue 686583  may report the same problem.
 

Comment 1 by ortuno@chromium.org, Sep 19 2016

Blocking: 490399
Status: Available (was: Unconfirmed)
Thanks for the detailed report!
We are seeing the same that might be the same or related to this bug, where we are running Zephyr on a arduino 101, that registers a couple BLE Gatt services, where one of the service allows client running Android device to subscribe to temperature updates.  We wrote our web page that uses the BLE api to connect to the device, and then monitors the updates, the device sends 1 notifcation every 0.5 second, and the bug we are seeing is that after 5-10 mins or maybe sometimes sooner, we will stop receiving notification.   We can tell that it's a Chromium bug because we use another Android app like nrfConnect or nrfToolbox and subscribe to the same service, and we cannot reproduce this, and we've verified that the device is continuing to push out notifcation over BLE, but the phone running Android has stopped receiving them.  As a work-around, we've modified our code to only send notifications if it changed in a certain threshold, and we put a gap of at least 2 seconds between notifcations.  Here's the link to github issue we've filed to track this in our project that might be helpful to reproduce, but it will require you to get a Arduino 101 board since we are running Zephyr on the device.

https://github.com/01org/zephyr.js/issues/148

Environment Setup:
Phone: Nexus 5x
OS: Android 7.0
Chromium Version: 53.0.2785.124

We suspect this is the same or similar bug, so instead of filing a new bug, I've decided to comment on this bug first to see if someone can look into this bug.

Comment 3 by ortuno@chromium.org, Oct 26 2016

#2: Thanks for the report! Seems to be a separate issue. I opened  http://crbug.com/659443 . Could you please take a look and answer the questions?

Comment 4 by ortuno@chromium.org, Nov 24 2016

Labels: Hotlist-GoodFirstBug

Comment 5 by scheib@chromium.org, Jan 30 2017

Labels: Notes-PartnerReported
Description: Show this description
To extract the characteristic value directly in the callback (i.e. on the Binder thread) will fix this bug. I have read that oneway aidl calls from a service app to the user app (which is used internally in BluetoothGatt) are serialized when they are performed on the same Binder object. That means a new notification (and any other callback) cannot arrive until a current callback has returned for that gatt object.

You can test this easily by simply doing a "sleep" in the notification callback. You will see that the next notification will not arrive until the current callback has returned.

Comment 8 by ortuno@chromium.org, Feb 20 2017

Owner: ortuno@chromium.org
Status: Started (was: Available)

Comment 9 by ortuno@chromium.org, Feb 20 2017

 Issue 686583  has been merged into this issue.
For info, https://github.com/pdjstone/cloudpets-web-bluetooth/#bugs-and-notes faced this issue as well. Hopefully it will be solved soon.
Labels: -Notes-PartnerReported
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afc9c240ef4f83bf23dfeec704f231f08c5c8121

commit afc9c240ef4f83bf23dfeec704f231f08c5c8121
Author: ortuno <ortuno@chromium.org>
Date: Tue Mar 07 05:21:31 2017

bluetooth: Save characteristic value when notification arrives

"Once a notification is arrived, work is posted to another thread which
then reads the latest characteristic value. If multiple notifications
arrive fast, this could result in race conditions where all the
onCharacteristicChanged callbacks are first triggered, then the three tasks
posted are executed which all extract the latest notified value instead of
the individual values." -- emil.len...@gmail.com

 [1] Post a task when sending GATT events http://crrev.com/2708513002
 [2.1] Close and null out BluetoothGatt on the UI Thread http://crrev.com/2702163002
 [2.2] Copy characteristic's value before posting task. (This patch).

BUG= 647673 

Review-Url: https://codereview.chromium.org/2707823002
Cr-Commit-Position: refs/heads/master@{#455037}

[modify] https://crrev.com/afc9c240ef4f83bf23dfeec704f231f08c5c8121/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java
[modify] https://crrev.com/afc9c240ef4f83bf23dfeec704f231f08c5c8121/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
[modify] https://crrev.com/afc9c240ef4f83bf23dfeec704f231f08c5c8121/device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc
[modify] https://crrev.com/afc9c240ef4f83bf23dfeec704f231f08c5c8121/device/bluetooth/test/test_bluetooth_adapter_observer.cc
[modify] https://crrev.com/afc9c240ef4f83bf23dfeec704f231f08c5c8121/device/bluetooth/test/test_bluetooth_adapter_observer.h

Status: Fixed (was: Started)
I'll try this out with my CloudPet when it lands in Canary (tomorrow, I presume?)
You can try it now with latest Chromium build as well at https://download-chromium.appspot.com/
Seems to have fixed the problem for me - I can now download audio (i.e. receive rapid notifications) without getting any duplicate or missed data.

Sign in to add a comment