Fix BT LE service addition timing issue on Android

If services were added in a tight loop sometimes the services are
created wrong. In practice this results in a situation where a client
reads a characteristic value from "Service A", but gets the value from
"Service B" - even if the client had no knowledge of "Service B".

The problem is that according to Android documentation, the
BluetoothGattServer::addService() must not be called before the prior
added service has received a
BluetoothGattServerCallback::onServiceAdded() -callback.

This commit serializes/queues the service additions.

Fixes: QTBUG-96742
Change-Id: I42c980600419787d4490d1a1059e3893597cb7cf
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
(cherry picked from commit 4deb789fe67615ebfa99af6f1071d20b0265a2e9)
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

1 files changed, 52 insertions, 2 deletions

diff --git a/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLEServer.java b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLEServer.java
index 22a7e36e..8dad964b 100644
--- a/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLEServer.java
+++ b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLEServer.java
@@ -82,6 +82,11 @@ public class QtBluetoothLEServer {
     private BluetoothGattServer mGattServer = null;
     private BluetoothLeAdvertiser mLeAdvertiser = null;
 
+    // Note: service additions -list is accessed from different threads as it is manipulated
+    // from Qt/JNI as well as from Android callbacks => synchronize the access
+    private ArrayList<BluetoothGattService> mPendingServiceAdditions =
+                      new ArrayList<BluetoothGattService>();
+
     private String mRemoteName = "";
     public String remoteName() { return mRemoteName; }
 
@@ -237,6 +242,10 @@ public class QtBluetoothLEServer {
                     qtControllerState = 0; // QLowEnergyController::UnconnectedState
                     clientCharacteristicManager.markDeviceConnectivity(device, false);
                     mGattServer.close();
+                    synchronized (mPendingServiceAdditions)
+                    {
+                        mPendingServiceAdditions.clear();
+                    }
                     mGattServer = null;
                     break;
                 case BluetoothProfile.STATE_CONNECTED:
@@ -264,6 +273,31 @@ public class QtBluetoothLEServer {
         @Override
         public void onServiceAdded(int status, BluetoothGattService service) {
             super.onServiceAdded(status, service);
+            Log.d(TAG, "Service " + service.getUuid().toString() + " addition result: " + status);
+
+            // Remove the indicated service from the pending queue
+            synchronized (mPendingServiceAdditions)
+            {
+                ListIterator<BluetoothGattService> iterator = mPendingServiceAdditions.listIterator();
+                while (iterator.hasNext()) {
+                    if (iterator.next().getUuid().equals(service.getUuid())) {
+                            iterator.remove();
+                            break;
+                    }
+                }
+
+                // If there are more services in the queue, add the next whose add initiation succeeds
+                iterator = mPendingServiceAdditions.listIterator();
+                while (iterator.hasNext()) {
+                    BluetoothGattService nextService = iterator.next();
+                    if (mGattServer.addService(nextService)) {
+                        break;
+                    } else {
+                        Log.w(TAG, "Adding service " + nextService.getUuid().toString() + " failed");
+                        iterator.remove();
+                    }
+                }
+            }
         }
 
         @Override
@@ -421,6 +455,10 @@ public class QtBluetoothLEServer {
         if (mGattServer == null)
             return;
 
+        synchronized (mPendingServiceAdditions)
+        {
+            mPendingServiceAdditions.clear();
+        }
         mGattServer.close();
         mGattServer = null;
 
@@ -462,8 +500,20 @@ public class QtBluetoothLEServer {
             return;
         }
 
-        boolean success = mGattServer.addService(service);
-        Log.w(TAG, "Services successfully added: " + success);
+        // When we add a service, we must wait for onServiceAdded callback before adding the
+        // next one. If the pending service queue is empty it means that there are no ongoing
+        // service additions => add the service to the server. If there are services in the
+        // queue it means there is an initiated addition ongoing, and we only add to the queue.
+        synchronized (mPendingServiceAdditions) {
+            if (mPendingServiceAdditions.isEmpty()) {
+                if (mGattServer.addService(service))
+                    mPendingServiceAdditions.add(service);
+                else
+                    Log.w(TAG, "Adding service " + service.getUuid().toString() + " failed.");
+            } else {
+                mPendingServiceAdditions.add(service);
+            }
+        }
     }
 
     /*