Fix crashing btchat example when selecting remote devicev5.4.1

The example immidiately destroys the QBluetoothServiceDiscoveryAgent
when the user selects a remote chat service from the remote selector
dialog. This may happen even when the scheduled QtConcurrent call
to runSdpScan() was still pending. The subsequent signal callback into
the deleted parent caused a crash.

Unfortunately QtConcurrent::run() returns a QFuture which does not
permit stopping the pending thread execution. Therefore the runSdpScan()
had to be rewritten using QProcess to properly destruct pending
calls.

Change-Id: I1ed5e147feb94a26240901a02d836056eddabbf6
Reviewed-by: Timur Pocheptsov <Timur.Pocheptsov@digia.com>
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

6 files changed, 69 insertions, 59 deletions

diff --git a/examples/bluetooth/btchat/main.cpp b/examples/bluetooth/btchat/main.cpp
index ac23a11d..ed362d23 100644
--- a/examples/bluetooth/btchat/main.cpp
+++ b/examples/bluetooth/btchat/main.cpp
@@ -41,9 +41,11 @@
 #include "chat.h"
 
 #include <QApplication>
+//#include <QtCore/QLoggingCategory>
 
 int main(int argc, char *argv[])
 {
+    //QLoggingCategory::setFilterRules(QStringLiteral("qt.bluetooth* = true"));
     QApplication app(argc, argv);
 
     Chat d;
diff --git a/examples/bluetooth/btchat/remoteselector.cpp b/examples/bluetooth/btchat/remoteselector.cpp
index 79dc0564..913988a2 100644
--- a/examples/bluetooth/btchat/remoteselector.cpp
+++ b/examples/bluetooth/btchat/remoteselector.cpp
@@ -136,6 +136,8 @@ void RemoteSelector::on_remoteDevices_itemActivated(QListWidgetItem *item)
 {
     qDebug() << "got click" << item->text();
     m_service = m_discoveredServices.value(item);
+    if (m_discoveryAgent->isActive())
+        m_discoveryAgent->stop();
 
     accept();
 }
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent.cpp b/src/bluetooth/qbluetoothservicediscoveryagent.cpp
index ef28ef82..7274780a 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent.cpp
+++ b/src/bluetooth/qbluetoothservicediscoveryagent.cpp
@@ -183,6 +183,11 @@ QBluetoothServiceDiscoveryAgent::QBluetoothServiceDiscoveryAgent(const QBluetoot
 
 QBluetoothServiceDiscoveryAgent::~QBluetoothServiceDiscoveryAgent()
 {
+    if (isActive()) {
+        disconnect(); //don't emit any signals due to stop()
+        stop();
+    }
+
     delete d_ptr;
 }
 
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent.h b/src/bluetooth/qbluetoothservicediscoveryagent.h
index 5e47ada0..b4f9d5f3 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent.h
+++ b/src/bluetooth/qbluetoothservicediscoveryagent.h
@@ -42,6 +42,10 @@
 #include <QtBluetooth/QBluetoothUuid>
 #include <QtBluetooth/QBluetoothDeviceDiscoveryAgent>
 
+#ifdef QT_BLUEZ_BLUETOOTH
+#include <QtCore/qprocess.h>
+#endif
+
 QT_BEGIN_NAMESPACE
 
 class QBluetoothAddress;
@@ -106,7 +110,7 @@ private:
 #ifdef QT_BLUEZ_BLUETOOTH
     Q_PRIVATE_SLOT(d_func(), void _q_discoveredServices(QDBusPendingCallWatcher*))
     Q_PRIVATE_SLOT(d_func(), void _q_createdDevice(QDBusPendingCallWatcher*))
-    Q_PRIVATE_SLOT(d_func(), void _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::Error, const QString &, const QStringList &))
+    Q_PRIVATE_SLOT(d_func(), void _q_sdpScannerDone(int,QProcess::ExitStatus))
 #endif
 #ifdef QT_ANDROID_BLUETOOTH
     Q_PRIVATE_SLOT(d_func(), void _q_processFetchedUuids(const QBluetoothAddress &address,
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent_bluez.cpp b/src/bluetooth/qbluetoothservicediscoveryagent_bluez.cpp
index d82a73a8..75efa37d 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent_bluez.cpp
+++ b/src/bluetooth/qbluetoothservicediscoveryagent_bluez.cpp
@@ -65,7 +65,7 @@ static inline void convertAddress(quint64 from, quint8 (&to)[6])
 QBluetoothServiceDiscoveryAgentPrivate::QBluetoothServiceDiscoveryAgentPrivate(const QBluetoothAddress &deviceAdapter)
 :   error(QBluetoothServiceDiscoveryAgent::NoError), m_deviceAdapterAddress(deviceAdapter), state(Inactive), deviceDiscoveryAgent(0),
     mode(QBluetoothServiceDiscoveryAgent::MinimalDiscovery), singleDevice(false),
-    manager(0), managerBluez5(0), adapter(0), device(0)
+    manager(0), managerBluez5(0), adapter(0), device(0), sdpScannerProcess(0)
 {
     if (isBluez5()) {
         managerBluez5 = new OrgFreedesktopDBusObjectManagerInterface(
@@ -136,6 +136,7 @@ void QBluetoothServiceDiscoveryAgentPrivate::start(const QBluetoothAddress &addr
 
 }
 
+// Bluez 5
 void QBluetoothServiceDiscoveryAgentPrivate::startBluez5(const QBluetoothAddress &address)
 {
     Q_Q(QBluetoothServiceDiscoveryAgent);
@@ -185,81 +186,64 @@ void QBluetoothServiceDiscoveryAgentPrivate::startBluez5(const QBluetoothAddress
     if (DiscoveryMode() == QBluetoothServiceDiscoveryAgent::MinimalDiscovery) {
         performMinimalServiceDiscovery(address);
     } else {
-        // we need to run the discovery in a different thread
-        // as it involves blocking calls
-        QtConcurrent::run(this, &QBluetoothServiceDiscoveryAgentPrivate::runSdpScan,
-                          address, QBluetoothAddress(adapter.address()));
+        runExternalSdpScan(address, QBluetoothAddress(adapter.address()));
     }
 }
 
-/*
- * This function runs in a different thread. We need to be very careful what we
- * access from here. That's why invokeMethod is used below.
- *
+/* Bluez 5
  * src/tools/sdpscanner performs an SDP scan. This is
  * done out-of-process to avoid license issues. At this stage Bluez uses GPLv2.
  */
-void QBluetoothServiceDiscoveryAgentPrivate::runSdpScan(
+void QBluetoothServiceDiscoveryAgentPrivate::runExternalSdpScan(
         const QBluetoothAddress &remoteAddress, const QBluetoothAddress localAddress)
 {
     Q_Q(QBluetoothServiceDiscoveryAgent);
 
-    const QString binPath = QLibraryInfo::location(QLibraryInfo::BinariesPath);
-
-    QFileInfo fileInfo(binPath, QStringLiteral("sdpscanner"));
-    if (!fileInfo.exists() || !fileInfo.isExecutable()) {
-        QMetaObject::invokeMethod(q, "_q_finishSdpScan", Qt::QueuedConnection,
-                                  Q_ARG(QBluetoothServiceDiscoveryAgent::Error,
-                                        QBluetoothServiceDiscoveryAgent::InputOutputError),
-                                  Q_ARG(QString,
-                                        QBluetoothServiceDiscoveryAgent::tr("Unable to find sdpscanner")),
-                                  Q_ARG(QStringList, QStringList()));
-        qCWarning(QT_BT_BLUEZ) << "Cannot find sdpscanner:"
-                               << fileInfo.canonicalFilePath();
-        return;
+    if (!sdpScannerProcess) {
+        const QString binPath = QLibraryInfo::location(QLibraryInfo::BinariesPath);
+        QFileInfo fileInfo(binPath, QStringLiteral("sdpscanner"));
+        if (!fileInfo.exists() || !fileInfo.isExecutable()) {
+            _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::InputOutputError,
+                             QBluetoothServiceDiscoveryAgent::tr("Unable to find sdpscanner"),
+                             QStringList());
+            qCWarning(QT_BT_BLUEZ) << "Cannot find sdpscanner:"
+                                   << fileInfo.canonicalFilePath();
+            return;
+        }
+
+        sdpScannerProcess = new QProcess(q);
+        sdpScannerProcess->setReadChannel(QProcess::StandardOutput);
+        sdpScannerProcess->setProgram(fileInfo.canonicalFilePath());
+        q->connect(sdpScannerProcess, SIGNAL(finished(int,QProcess::ExitStatus)),
+                   q, SLOT(_q_sdpScannerDone(int,QProcess::ExitStatus)));
+
     }
 
     QStringList arguments;
     arguments << remoteAddress.toString() << localAddress.toString();
 
-    QByteArray output;
-
-    QProcess process;
-    process.setProcessChannelMode(QProcess::ForwardedErrorChannel);
-    process.setReadChannel(QProcess::StandardOutput);
-    process.start(fileInfo.canonicalFilePath(), arguments);
-
-    if (process.waitForStarted(-1)) {
-        while (process.waitForReadyRead(-1))
-            output += process.readAllStandardOutput();
-    }
-
-    process.waitForFinished();
+    sdpScannerProcess->setArguments(arguments);
+    sdpScannerProcess->start();
+}
 
-    if (process.exitStatus() != QProcess::NormalExit
-            || process.exitCode() != 0) {
-        qCWarning(QT_BT_BLUEZ) << "SDP scan failure"
-                               << process.exitStatus() << process.exitCode()
-                               << remoteAddress;
+// Bluez 5
+void QBluetoothServiceDiscoveryAgentPrivate::_q_sdpScannerDone(int exitCode, QProcess::ExitStatus status)
+{
+    if (status != QProcess::NormalExit || exitCode != 0) {
+        qCWarning(QT_BT_BLUEZ) << "SDP scan failure" << status << exitCode;
         if (singleDevice) {
-            QMetaObject::invokeMethod(q, "_q_finishSdpScan", Qt::QueuedConnection,
-                                  Q_ARG(QBluetoothServiceDiscoveryAgent::Error,
-                                        QBluetoothServiceDiscoveryAgent::InputOutputError),
-                                  Q_ARG(QString,
-                                        QBluetoothServiceDiscoveryAgent::tr("Unable to perform SDP scan")),
-                                  Q_ARG(QStringList, QStringList()));
+            _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::InputOutputError,
+                             QBluetoothServiceDiscoveryAgent::tr("Unable to perform SDP scan"),
+                             QStringList());
         } else {
             // go to next device
-            QMetaObject::invokeMethod(q, "_q_finishSdpScan", Qt::QueuedConnection,
-                                      Q_ARG(QBluetoothServiceDiscoveryAgent::Error,
-                                            QBluetoothServiceDiscoveryAgent::NoError),
-                                      Q_ARG(QString, QString()),
-                                      Q_ARG(QStringList, QStringList()));
+            _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::NoError, QString(), QStringList());
         }
         return;
     }
 
     QStringList xmlRecords;
+    const QByteArray output = sdpScannerProcess->readAllStandardOutput();
     const QString decodedData = QString::fromUtf8(QByteArray::fromBase64(output));
 
     // split the various xml docs up
@@ -276,13 +260,10 @@ void QBluetoothServiceDiscoveryAgentPrivate::runSdpScan(
         } while ( start != -1);
     }
 
-    QMetaObject::invokeMethod(q, "_q_finishSdpScan", Qt::QueuedConnection,
-                              Q_ARG(QBluetoothServiceDiscoveryAgent::Error,
-                                    QBluetoothServiceDiscoveryAgent::NoError),
-                              Q_ARG(QString, QString()),
-                              Q_ARG(QStringList, xmlRecords));
+    _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::NoError, QString(), xmlRecords);
 }
 
+// Bluez 5
 void QBluetoothServiceDiscoveryAgentPrivate::_q_finishSdpScan(QBluetoothServiceDiscoveryAgent::Error errorCode,
                                                               const QString &errorDescription,
                                                               const QStringList &xmlRecords)
@@ -352,8 +333,19 @@ void QBluetoothServiceDiscoveryAgentPrivate::stop()
         Q_ASSERT(!device);
     }
 
+
     discoveredDevices.clear();
     setDiscoveryState(Inactive);
+
+    // must happen after discoveredDevices.clear() above to avoid retrigger of next scan
+    // while waitForFinished() is waiting
+    if (sdpScannerProcess) { // Bluez 5
+        if (sdpScannerProcess->state() != QProcess::NotRunning) {
+            sdpScannerProcess->kill();
+            sdpScannerProcess->waitForFinished();
+        }
+    }
+
     Q_Q(QBluetoothServiceDiscoveryAgent);
     emit q->canceled();
 }
@@ -591,6 +583,7 @@ QBluetoothServiceInfo QBluetoothServiceDiscoveryAgentPrivate::parseServiceXml(
     return serviceInfo;
 }
 
+// Bluez 5
 void QBluetoothServiceDiscoveryAgentPrivate::performMinimalServiceDiscovery(const QBluetoothAddress &deviceAddress)
 {
     if (foundHostAdapterPath.isEmpty()) {
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent_p.h b/src/bluetooth/qbluetoothservicediscoveryagent_p.h
index ea985627..3b9c0a42 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent_p.h
+++ b/src/bluetooth/qbluetoothservicediscoveryagent_p.h
@@ -58,6 +58,7 @@ class OrgBluezManagerInterface;
 class OrgBluezAdapterInterface;
 class OrgBluezDeviceInterface;
 class OrgFreedesktopDBusObjectManagerInterface;
+#include <QtCore/qprocess.h>
 
 QT_BEGIN_NAMESPACE
 class QDBusPendingCallWatcher;
@@ -127,6 +128,7 @@ public:
     void _q_discoverGattCharacteristics(QDBusPendingCallWatcher *watcher);
     void _q_discoveredGattCharacteristic(QDBusPendingCallWatcher *watcher);
     */
+    void _q_sdpScannerDone(int exitCode, QProcess::ExitStatus status);
     void _q_finishSdpScan(QBluetoothServiceDiscoveryAgent::Error errorCode,
                           const QString &errorDescription,
                           const QStringList &xmlRecords);
@@ -147,8 +149,9 @@ private:
 
 #ifdef QT_BLUEZ_BLUETOOTH
     void startBluez5(const QBluetoothAddress &address);
-    void runSdpScan(const QBluetoothAddress &remoteAddress,
+    void runExternalSdpScan(const QBluetoothAddress &remoteAddress,
                     const QBluetoothAddress localAddress);
+    void sdpScannerDone(int exitCode, QProcess::ExitStatus exitStatus);
     QVariant readAttributeValue(QXmlStreamReader &xml);
     QBluetoothServiceInfo parseServiceXml(const QString& xml);
     void performMinimalServiceDiscovery(const QBluetoothAddress &deviceAddress);
@@ -195,6 +198,7 @@ private:
     OrgFreedesktopDBusObjectManagerInterface *managerBluez5;
     OrgBluezAdapterInterface *adapter;
     OrgBluezDeviceInterface *device;
+    QProcess *sdpScannerProcess;
 #endif
 
 #ifdef QT_ANDROID_BLUETOOTH