QLowEnergyController Windows: fix UI hangs during discovery on disconnected paired devices

Commit 0b60ca266f0fe27053a58eff3dbd903e3a1678ca introduced Close()
calls for GattDeviceService objects. IIRC, it was done to avoid
potential GattCommunicationStatus_AccessDenied errors, which could
otherwise happen when trying to re-acquire the non-closed service.
This commit does the proper cleanup when the device is disconnected.

However there is one corner-case: when we try to connect to a paired
device which is turned off, Windows provides its services (using some
cached data), but the service details discovery fails.
The details discovery is executed in a background thread using a set
of async calls. If the user disconnects from the device during this
discovery, the GattDeviceService::Close() call for the respective
service will block until the async operation fails (which takes
a couple of seconds on Windows 11). As a result, the UI thread freezes
for this time.

This patch is an attempt to avoid it by shifting the Close() call to
the background helper thread in such cases. If there is an error during
details discovery, or if it just takes too long, and the user decides
to disconnect from the device, the service will be closed directly
in the helper thread.
However, if the details discovery is completed successfully, the
GattDeviceService object can still be re-used for other operations,
and it will not be deleted from cache until the device is disconnected.

Fixes: QTBUG-108461
Change-Id: I8ebe945130808ed7bd8852a76bba84c73651f5a5
Reviewed-by: Juha Vuolle <juha.vuolle@qt.io>
(cherry picked from commit 4c92565f04755eb011be4b893d2619e25fb008c4)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 20 insertions, 12 deletions

diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp
index 8c61b4ee..af49e642 100644
--- a/src/bluetooth/qlowenergycontroller_winrt.cpp
+++ b/src/bluetooth/qlowenergycontroller_winrt.cpp
@@ -102,13 +102,14 @@ static QByteArray byteArrayFromGattResult(const ComPtr<IGattReadResult> &gattRes
     return byteArrayFromBuffer(buffer, isWCharString);
 }
 
-static void closeDeviceService(ComPtr<IGattDeviceService> service)
+template <typename T>
+static void closeDeviceService(ComPtr<T> service)
 {
     ComPtr<IClosable> closableService;
     HRESULT hr = service.As(&closableService);
-    RETURN_IF_FAILED("Could not cast service to closable", return);
+    RETURN_IF_FAILED("Could not cast type to closable", return);
     hr = closableService->Close();
-    RETURN_IF_FAILED("Service Close() failed", return);
+    RETURN_IF_FAILED("Close() call failed", return);
 }
 
 class QWinRTLowEnergyServiceHandler : public QObject
@@ -125,6 +126,8 @@ public:
 
     ~QWinRTLowEnergyServiceHandler()
     {
+        if (mAbortRequested)
+            closeDeviceService(mDeviceService);
         qCDebug(QT_BT_WINDOWS) << __FUNCTION__;
     }
 
@@ -323,7 +326,7 @@ public slots:
     }
 
 private:
-    bool checkAllCharacteristicsDiscovered();
+    void checkAllCharacteristicsDiscovered();
     void emitErrorAndQuitThread(HRESULT hr);
     void emitErrorAndQuitThread(const QString &error);
 
@@ -346,18 +349,13 @@ signals:
     void errorOccured(const QString &error);
 };
 
-bool QWinRTLowEnergyServiceHandler::checkAllCharacteristicsDiscovered()
+void QWinRTLowEnergyServiceHandler::checkAllCharacteristicsDiscovered()
 {
-    if (mCharacteristicsCountToBeDiscovered == 0) {
+    if (!mAbortRequested && (mCharacteristicsCountToBeDiscovered == 0)) {
         emit charListObtained(mService, mCharacteristicList, mIndicateChars,
                               mStartHandle, mEndHandle);
-        QThread::currentThread()->quit();
-        return true;
-    } else if (mAbortRequested) {
-        QThread::currentThread()->quit();
     }
-
-    return false;
+    QThread::currentThread()->quit();
 }
 
 void QWinRTLowEnergyServiceHandler::emitErrorAndQuitThread(HRESULT hr)
@@ -367,6 +365,7 @@ void QWinRTLowEnergyServiceHandler::emitErrorAndQuitThread(HRESULT hr)
 
 void QWinRTLowEnergyServiceHandler::emitErrorAndQuitThread(const QString &error)
 {
+    mAbortRequested = true; // so that the service is closed during cleanup
     emit errorOccured(error);
     QThread::currentThread()->quit();
 }
@@ -1158,6 +1157,12 @@ HRESULT QLowEnergyControllerPrivateWinRT::onServiceDiscoveryFinished(ABI::Window
 
 void QLowEnergyControllerPrivateWinRT::clearAllServices()
 {
+    // These services will be closed in the respective
+    // QWinRTLowEnergyServiceHandler workers (in background threads).
+    for (auto &uuid : m_requestDetailsServiceUuids)
+        m_openedServices.remove(uuid);
+    m_requestDetailsServiceUuids.clear();
+
     for (auto service : m_openedServices) {
         closeDeviceService(service);
     }
@@ -1295,6 +1300,7 @@ void QLowEnergyControllerPrivateWinRT::discoverServiceDetailsHelper(
 
     QWinRTLowEnergyServiceHandler *worker =
             new QWinRTLowEnergyServiceHandler(service, deviceService3, mode);
+    m_requestDetailsServiceUuids.insert(service);
     QThread *thread = new QThread;
     worker->moveToThread(thread);
     connect(thread, &QThread::started, worker, &QWinRTLowEnergyServiceHandler::obtainCharList);
@@ -1313,6 +1319,7 @@ void QLowEnergyControllerPrivateWinRT::discoverServiceDetailsHelper(
                     << "Discovery complete for unknown service:" << service.toString();
             return;
         }
+        m_requestDetailsServiceUuids.remove(service);
 
         QSharedPointer<QLowEnergyServicePrivate> pointer = serviceList.value(service);
         pointer->startHandle = startHandle;
diff --git a/src/bluetooth/qlowenergycontroller_winrt_p.h b/src/bluetooth/qlowenergycontroller_winrt_p.h
index 77a7716d..dc9c3235 100644
--- a/src/bluetooth/qlowenergycontroller_winrt_p.h
+++ b/src/bluetooth/qlowenergycontroller_winrt_p.h
@@ -126,6 +126,7 @@ private:
 
     using GattDeviceServiceComPtr = Microsoft::WRL::ComPtr<ABI::Windows::Devices::Bluetooth::GenericAttributeProfile::IGattDeviceService>;
     QMap<QBluetoothUuid, GattDeviceServiceComPtr> m_openedServices;
+    QSet<QBluetoothUuid> m_requestDetailsServiceUuids;
 
     using NativeServiceCallback = std::function<void(GattDeviceServiceComPtr)>;
     HRESULT getNativeService(const QBluetoothUuid &serviceUuid, NativeServiceCallback callback);