From 9cc6a9b3af559004bbfdfec3aa7dd258bee1eb77 Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Mon, 26 Jan 2015 08:47:06 +0100 Subject: Fix crashing btchat example when selecting remote device 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 Reviewed-by: Alex Blasche --- examples/bluetooth/btchat/main.cpp | 2 + examples/bluetooth/btchat/remoteselector.cpp | 2 + src/bluetooth/qbluetoothservicediscoveryagent.cpp | 5 + src/bluetooth/qbluetoothservicediscoveryagent.h | 6 +- .../qbluetoothservicediscoveryagent_bluez.cpp | 107 ++++++++++----------- src/bluetooth/qbluetoothservicediscoveryagent_p.h | 6 +- 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 +//#include 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 #include +#ifdef QT_BLUEZ_BLUETOOTH +#include +#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 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 -- cgit v1.2.3