From bcc8988b4be771d5bb38811591830732a4a316f9 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Tue, 20 Dec 2022 16:31:27 +0100 Subject: 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 (cherry picked from commit 4c92565f04755eb011be4b893d2619e25fb008c4) Reviewed-by: Qt Cherry-pick Bot --- src/bluetooth/qlowenergycontroller_winrt.cpp | 31 +++++++++++++++++----------- src/bluetooth/qlowenergycontroller_winrt_p.h | 1 + 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp index 3822ed3a..9fefee2a 100644 --- a/src/bluetooth/qlowenergycontroller_winrt.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt.cpp @@ -102,13 +102,14 @@ static QByteArray byteArrayFromGattResult(const ComPtr &gattRes return byteArrayFromBuffer(buffer, isWCharString); } -static void closeDeviceService(ComPtr service) +template +static void closeDeviceService(ComPtr service) { ComPtr 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 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; QMap m_openedServices; + QSet m_requestDetailsServiceUuids; using NativeServiceCallback = std::function; HRESULT getNativeService(const QBluetoothUuid &serviceUuid, NativeServiceCallback callback); -- cgit v1.2.1