Properly invalidate services when calling disconnectService() on BlueZ

While the central role implementation properly invalidates all the
LowEnergyServicePrivate instances, peripheral mode was leaking service
instances. This is triggered when the peripheral disconnects from a client
or when the user calls disconnectService(). On the other hand
stopAdvertising() does not do that.

This patch fixes the service instance leak and ensures that the class
docs specifically state the behavior difference between stopAdvertising()
and disconnectService().

Change-Id: Ia52b141096dc1db3d0cefe3ed18c230eecccd9c0
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>

3 files changed, 28 insertions, 7 deletions

diff --git a/src/bluetooth/qlowenergycontroller.cpp b/src/bluetooth/qlowenergycontroller.cpp
index a23955ff..6b4c17d6 100644
--- a/src/bluetooth/qlowenergycontroller.cpp
+++ b/src/bluetooth/qlowenergycontroller.cpp
@@ -603,9 +603,10 @@ void QLowEnergyController::connectToDevice()
 
     This function does nothing if the controller is in the \l UnconnectedState.
 
-    If the controller is in the peripheral role, it stops advertising too.
-    The application must restart the advertising mode by calling
-    \l startAdvertising().
+    If the controller is in the peripheral role, it stops advertising and removes
+    all services which have previously been added via \l addService().
+    To reuse the QLowEnergyController instance the application must re-add services
+    and restart the advertising mode by calling \l startAdvertising().
 
     \sa connectToDevice()
  */
@@ -742,6 +743,9 @@ void QLowEnergyController::startAdvertising(const QLowEnergyAdvertisingParameter
 /*!
    Stops advertising, if this object is currently in the advertising state.
 
+   The controller has to be in the \l PeripheralRole for this function to work.
+   It does not invalidate services which have previously been added via \l addService().
+
    \since 5.7
    \sa startAdvertising()
  */
@@ -760,8 +764,15 @@ void QLowEnergyController::stopAdvertising()
   The controller must be in the \l PeripheralRole and in the \l UnconnectedState. The \a service
   object must be valid.
 
+  \note Once the peripheral instance is disconnected from the remote central device or
+  if \l disconnectFromDevice() is manually called, every service definition that was
+  previously added via this function is removed from the peripheral. Therefore this function
+  must be called again before re-advertising this peripheral controller instance. The described
+  behavior is connection specific and therefore not dependent on whether \l stopAdvertising()
+  was called.
+
   \since 5.7
-  \sa QLowEnergyServiceData::addIncludedService
+  \sa stopAdvertising(), disconnectFromDevice(), QLowEnergyServiceData::addIncludedService
  */
 QLowEnergyService *QLowEnergyController::addService(const QLowEnergyServiceData &service,
                                                     QObject *parent)
diff --git a/src/bluetooth/qlowenergycontroller_bluez.cpp b/src/bluetooth/qlowenergycontroller_bluez.cpp
index d58e0ee8..d288f3ad 100644
--- a/src/bluetooth/qlowenergycontroller_bluez.cpp
+++ b/src/bluetooth/qlowenergycontroller_bluez.cpp
@@ -809,9 +809,12 @@ void QLowEnergyControllerPrivateBluez::resetController()
     securityLevelValue = -1;
     connectionHandle = 0;
 
-    // public API behavior requires stop of advertisement
-    if (role == QLowEnergyController::PeripheralRole && advertiser)
-        advertiser->stopAdvertising();
+    if (role == QLowEnergyController::PeripheralRole) {
+        // public API behavior requires stop of advertisement
+        if (advertiser)
+            advertiser->stopAdvertising();
+        localAttributes.clear();
+    }
 }
 
 void QLowEnergyControllerPrivateBluez::restartRequestTimer()
diff --git a/src/bluetooth/qlowenergycontrollerbase.cpp b/src/bluetooth/qlowenergycontrollerbase.cpp
index 687f5b9e..c7b9ec0b 100644
--- a/src/bluetooth/qlowenergycontrollerbase.cpp
+++ b/src/bluetooth/qlowenergycontrollerbase.cpp
@@ -261,6 +261,8 @@ void QLowEnergyControllerPrivate::invalidateServices()
     }
 
     serviceList.clear();
+    localServices.clear();
+    lastLocalHandle = {};
 }
 
 QLowEnergyService *QLowEnergyControllerPrivate::addServiceHelper(
@@ -308,7 +310,12 @@ QLowEnergyService *QLowEnergyControllerPrivate::addServiceHelper(
         return nullptr;
     }
 
+    if (localServices.contains(servicePrivate->uuid)) {
+        qWarning() << "Overriding existing local service with uuid"
+                   << servicePrivate->uuid;
+    }
     this->localServices.insert(servicePrivate->uuid, servicePrivate);
+
     this->addToGenericAttributeList(service, servicePrivate->startHandle);
     return new QLowEnergyService(servicePrivate);
 }