Windows device discovery: fix crash at discovery stop

The crash is cannot be reproduced reliably.
One of the possible scenarios is:
* After finishing the scanning a last
  QWinRTBluetoothDeviceDiscoveryWorker::onAdvertisementDataReceived()
  slot is executed on worker's thread (thread A).
* This slot starts an async BluetoothLEDevice::FromBluetoothAddressAsync()
  call, capturing shared_from_this() to make sure that the worker object
  is alive when the async call completes. The async call executes on
  some other thread B.
* The async call completes on thread B with a failure (that happens with
  some LE devices), so we call
  invokeDecrementPendingDevicesCountAndCheckFinished(). This call
  schedules a decrementPendingDevicesCountAndCheckFinished() function
  call on thread A.
* The lambda in thread B is completed, so shared pointer's counter is
  decremented. It is the last instance of the worker, so its destructor
  is called.
* At the same time the decrementPendingDevicesCountAndCheckFinished()
  function is called on thread A.
* It refers to an already deleted worker -> crash.

This patch fixes it by passing the shared worker instance to the
decrementPendingDevicesCountAndCheckFinished() function, making sure
that the worker object is alive when this function is invoked.

As a drive-by: get rid of the {invoke}IncrementPendingDevicesCount()
functions, because we always increment the device count from the
worker thread, so we can update the variable directly.

This commit amends 2f560d044fec92e94e8438791aa5e4d9daced197 and
36dd802c964f97522d1f5a75c8fb7a67f3061a3d.

Fixes: QTBUG-106029
Change-Id: I2d82c34b17c8cef873c9c61a92d874c377501edb
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
Reviewed-by: Juha Vuolle <juha.vuolle@insta.fi>
(cherry picked from commit e284887e0f6093767a5af16d497549860ca1770d)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

1 files changed, 26 insertions, 27 deletions

diff --git a/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp b/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp
index 44932e1d..7fceb041 100644
--- a/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp
+++ b/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp
@@ -230,8 +230,8 @@ private:
     std::shared_ptr<AdvertisementWatcherWrapper> createAdvertisementWatcher();
 
     // invokable methods for handling finish conditions
-    Q_INVOKABLE void incrementPendingDevicesCount();
-    Q_INVOKABLE void decrementPendingDevicesCountAndCheckFinished();
+    Q_INVOKABLE void decrementPendingDevicesCountAndCheckFinished(
+            std::shared_ptr<QWinRTBluetoothDeviceDiscoveryWorker> worker);
 
 Q_SIGNALS:
     void deviceFound(const QBluetoothDeviceInfo &info);
@@ -275,15 +275,13 @@ private:
     QTimer *m_leScanTimer = nullptr;
 };
 
-static void invokeIncrementPendingDevicesCount(QObject *context)
+static void invokeDecrementPendingDevicesCountAndCheckFinished(
+        std::shared_ptr<QWinRTBluetoothDeviceDiscoveryWorker> worker)
 {
-    QMetaObject::invokeMethod(context, "incrementPendingDevicesCount", Qt::QueuedConnection);
-}
-
-static void invokeDecrementPendingDevicesCountAndCheckFinished(QObject *context)
-{
-    QMetaObject::invokeMethod(context, "decrementPendingDevicesCountAndCheckFinished",
-                              Qt::QueuedConnection);
+    QMetaObject::invokeMethod(worker.get(), "decrementPendingDevicesCountAndCheckFinished",
+                              Qt::QueuedConnection,
+                              Q_ARG(std::shared_ptr<QWinRTBluetoothDeviceDiscoveryWorker>,
+                                    worker));
 }
 
 QWinRTBluetoothDeviceDiscoveryWorker::QWinRTBluetoothDeviceDiscoveryWorker(
@@ -451,7 +449,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::onAdvertisementDataReceived(
             m_foundLEDevicesMap.insert(address, info);
         }
     }
-    invokeIncrementPendingDevicesCount(this); // as if we discovered a new LE device
+    ++m_pendingDevices; // as if we discovered a new LE device
     auto thisPtr = shared_from_this();
     auto asyncOp = BluetoothLEDevice::FromBluetoothAddressAsync(address);
     asyncOp.Completed([thisPtr, address](auto &&op, AsyncStatus status) {
@@ -466,7 +464,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::onAdvertisementDataReceived(
             // status != Completed or failed to extract result
             qCDebug(QT_BT_WINDOWS) << "Failed to get LE device from address"
                                    << QBluetoothAddress(address);
-            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr.get());
+            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr);
         }
     });
 }
@@ -529,7 +527,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::getClassicDeviceFromId(const winrt::h
             }
             // status != Completed or failed to extract result
             qCDebug(QT_BT_WINDOWS) << "Failed to get Classic device from id";
-            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr.get());
+            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr);
         }
     });
 }
@@ -554,7 +552,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleClassicDevice(const BluetoothDe
             }
             // Failed to get services
             qCDebug(QT_BT_WINDOWS) << "Failed to get RFCOMM services for device" << btName;
-            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr.get());
+            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr);
         }
     });
 }
@@ -565,8 +563,9 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleRfcommServices(
         const QString &name, uint32_t classOfDeviceInt)
 {
     // need to perform the check even if some of the operations fails
-    auto guard = qScopeGuard([this]() {
-        invokeDecrementPendingDevicesCountAndCheckFinished(this);
+    auto shared = shared_from_this();
+    auto guard = qScopeGuard([shared]() {
+        invokeDecrementPendingDevicesCountAndCheckFinished(shared);
     });
     Q_UNUSED(guard); // to suppress warning
 
@@ -599,16 +598,15 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleRfcommServices(
                               Q_ARG(QBluetoothDeviceInfo, info));
 }
 
-void QWinRTBluetoothDeviceDiscoveryWorker::incrementPendingDevicesCount()
-{
-    ++m_pendingDevices;
-}
-
-void QWinRTBluetoothDeviceDiscoveryWorker::decrementPendingDevicesCountAndCheckFinished()
+void QWinRTBluetoothDeviceDiscoveryWorker::decrementPendingDevicesCountAndCheckFinished(
+        std::shared_ptr<QWinRTBluetoothDeviceDiscoveryWorker> worker)
 {
     --m_pendingDevices;
     if (isFinished())
         finishDiscovery();
+    // Worker is passed here simply to make sure that the object is still alive
+    // when we call this method via QObject::invoke().
+    Q_UNUSED(worker)
 }
 
 // this function executes in main worker thread
@@ -628,7 +626,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::getLowEnergyDeviceFromId(const winrt:
             }
             // status != Completed or failed to extract result
             qCDebug(QT_BT_WINDOWS) << "Failed to get LE device from id";
-            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr.get());
+            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr);
         }
     });
 }
@@ -660,7 +658,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleLowEnergyDevice(const Bluetooth
     // Use the services obtained from the advertisement data if the device is not paired
     if (!isPaired) {
         info.setServiceUuids(adInfo.services);
-        invokeDecrementPendingDevicesCountAndCheckFinished(this);
+        invokeDecrementPendingDevicesCountAndCheckFinished(shared_from_this());
         invokeDeviceFoundWithDebug(info);
     } else {
         auto asyncOp = device.GetGattServicesAsync();
@@ -675,7 +673,7 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleLowEnergyDevice(const Bluetooth
             }
             // Failed to get services
             qCDebug(QT_BT_WINDOWS) << "Failed to get GATT services for device" << info.name();
-            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr.get());
+            invokeDecrementPendingDevicesCountAndCheckFinished(thisPtr);
         });
     }
 }
@@ -685,8 +683,9 @@ void QWinRTBluetoothDeviceDiscoveryWorker::handleGattServices(
         const GattDeviceServicesResult &servicesResult, QBluetoothDeviceInfo &info)
 {
     // need to perform the check even if some of the operations fails
-    auto guard = qScopeGuard([this]() {
-        invokeDecrementPendingDevicesCountAndCheckFinished(this);
+    auto shared = shared_from_this();
+    auto guard = qScopeGuard([shared]() {
+        invokeDecrementPendingDevicesCountAndCheckFinished(shared);
     });
     Q_UNUSED(guard); // to suppress warning