Close the GattDeviceService once we are done with it

The Windows implementation is missing a significant detail.
According to the docs, we need to call a Close() method when we are
done working with the GattDeviceService, so that its resources are
freed.

In practice that is not so easily achievable, because the lifetime
of the GattDeviceService instance can't be easily controlled due
to various async operations. Apart from that, we subscribe for
value changes of the characteristics that can notify.

As a result, the most reasonable solution is to implement a cache
of opened GattDeviceService instances. Objects from this cache are
reused each time we need to perform some operations on the service.
The cache is cleared when we disconnect from the device. Apart from
that we need to remove the specific instances from the cache when
we rediscover services and their details.

Task-number: QTBUG-94001
Change-Id: I9f7cc55d7972591715e33081db6e43a97d5f0b67
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
Reviewed-by: Juha Vuolle <juha.vuolle@insta.fi>
(cherry picked from commit 0b60ca266f0fe27053a58eff3dbd903e3a1678ca)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 41 insertions, 3 deletions

diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp
index b33d67f9..1553429f 100644
--- a/src/bluetooth/qlowenergycontroller_winrt.cpp
+++ b/src/bluetooth/qlowenergycontroller_winrt.cpp
@@ -138,6 +138,15 @@ static QByteArray byteArrayFromGattResult(const ComPtr<IGattReadResult> &gattRes
     return byteArrayFromBuffer(buffer, isWCharString);
 }
 
+static void closeDeviceService(ComPtr<IGattDeviceService> service)
+{
+    ComPtr<IClosable> closableService;
+    HRESULT hr = service.As(&closableService);
+    RETURN_IF_FAILED("Could not cast service to closable", return);
+    hr = closableService->Close();
+    RETURN_IF_FAILED("Service Close() failed", return);
+}
+
 class QWinRTLowEnergyServiceHandler : public QObject
 {
     Q_OBJECT
@@ -735,6 +744,7 @@ void QLowEnergyControllerPrivateWinRT::disconnectFromDevice()
     unregisterFromValueChanges();
     unregisterFromStatusChanges();
     unregisterFromMtuChanges();
+    clearAllServices();
     mGattSession = nullptr;
     mDevice = nullptr;
     setState(QLowEnergyController::UnconnectedState);
@@ -744,11 +754,16 @@ void QLowEnergyControllerPrivateWinRT::disconnectFromDevice()
 ComPtr<IGattDeviceService> QLowEnergyControllerPrivateWinRT::getNativeService(
         const QBluetoothUuid &serviceUuid)
 {
+    if (m_openedServices.contains(serviceUuid))
+        return m_openedServices.value(serviceUuid);
+
     ComPtr<IGattDeviceService> deviceService;
     HRESULT hr;
     hr = mDevice->GetGattService(serviceUuid, &deviceService);
     if (FAILED(hr))
         qCDebug(QT_BT_WINDOWS) << "Could not obtain native service for Uuid" << serviceUuid;
+    if (deviceService)
+        m_openedServices[serviceUuid] = deviceService;
     return deviceService;
 }
 
@@ -761,7 +776,7 @@ ComPtr<IGattCharacteristic> QLowEnergyControllerPrivateWinRT::getNativeCharacter
 
     ComPtr<IGattDeviceService3> service3;
     HRESULT hr = service.As(&service3);
-    RETURN_IF_FAILED("Could not cast service", return nullptr);
+    RETURN_IF_FAILED("Could not cast service to service3", return nullptr);
 
     ComPtr<IAsyncOperation<GattCharacteristicsResult *>> op;
     ComPtr<IGattCharacteristicsResult> result;
@@ -1079,10 +1094,26 @@ HRESULT QLowEnergyControllerPrivateWinRT::onServiceDiscoveryFinished(ABI::Window
     return S_OK;
 }
 
+void QLowEnergyControllerPrivateWinRT::clearAllServices()
+{
+    for (auto service : m_openedServices) {
+        closeDeviceService(service);
+    }
+    m_openedServices.clear();
+}
+
+void QLowEnergyControllerPrivateWinRT::closeAndRemoveService(const QBluetoothUuid &uuid)
+{
+    auto service = m_openedServices.take(uuid);
+    if (service)
+        closeDeviceService(service);
+}
+
 void QLowEnergyControllerPrivateWinRT::discoverServices()
 {
     qCDebug(QT_BT_WINDOWS) << "Service discovery initiated";
-
+    // clear the previous services cache, as we request the services again
+    clearAllServices();
     ComPtr<IBluetoothLEDevice3> device3;
     HRESULT hr = mDevice.As(&device3);
     CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast device", return);
@@ -1106,6 +1137,9 @@ void QLowEnergyControllerPrivateWinRT::discoverServiceDetails(
         return;
     }
 
+    // clear the cache to rediscover service details
+    closeAndRemoveService(service);
+
     ComPtr<IGattDeviceService> deviceService = getNativeService(service);
     if (!deviceService) {
         qCDebug(QT_BT_WINDOWS) << "Could not obtain native service for uuid " << service;
@@ -1113,7 +1147,7 @@ void QLowEnergyControllerPrivateWinRT::discoverServiceDetails(
     }
 
     auto reactOnDiscoveryError = [](QSharedPointer<QLowEnergyServicePrivate> service,
-            const QString &msg)
+                                    const QString &msg)
     {
         qCDebug(QT_BT_WINDOWS) << msg;
         service->setError(QLowEnergyService::UnknownError);
diff --git a/src/bluetooth/qlowenergycontroller_winrt_p.h b/src/bluetooth/qlowenergycontroller_winrt_p.h
index 44c20cc8..91feace9 100644
--- a/src/bluetooth/qlowenergycontroller_winrt_p.h
+++ b/src/bluetooth/qlowenergycontroller_winrt_p.h
@@ -159,6 +159,7 @@ private:
         EventRegistrationToken token;
     };
     QList<ValueChangedEntry> mValueChangedTokens;
+    QMap<QBluetoothUuid, Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattDeviceService>> m_openedServices;
 
     Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattDeviceService> getNativeService(const QBluetoothUuid &serviceUuid);
     Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattCharacteristic> getNativeCharacteristic(const QBluetoothUuid &serviceUuid, const QBluetoothUuid &charUuid);
@@ -180,6 +181,9 @@ private:
         Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattDeviceService> nativeService);
     HRESULT onServiceDiscoveryFinished(ABI::Windows::Foundation::IAsyncOperation<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::GattDeviceServicesResult *> *op,
                                        ABI::Windows::Foundation::AsyncStatus status);
+
+    void clearAllServices();
+    void closeAndRemoveService(const QBluetoothUuid &uuid);
 };
 
 QT_END_NAMESPACE