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>

1 files changed, 50 insertions, 57 deletions

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()) {