From f6b30722fa166fa1ed9ea0ad14bdb6e757317932 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Wed, 1 Jun 2011 16:35:43 +0100 Subject: [PATCH] bearer: run the bearer engines in their own worker thread The original architecture of the QtNetwork bearer support hosted the engines in the application's main thread, but this causes some problems. If the QNetworkConfigurationManager is constructed in a worker thread, then it is populated asynchronously without any notification when it is done (the app gets incomplete or missing results) Fixing that by restoring the earlier behaviour of using blocking queued connections to wait for the lists to be populated caused a regression, as some applications deadlock because the main thread is waiting on the worker thread at this time. By introducing a dedicated worker thread for the bearer engines, QNetworkConfigurationManager can be safely constructed in any thread while using blocking queued connections internally. Task-number: QTBUG-18795 Change-Id: Iaa1706d44b02b42057c100b0b399364175af2ddb Reviewed-by: mread (cherry picked from commit 5f879c55e531165cc2569b03c3796d0f33d0a0b7) Reviewed-by: Jonas Gastal Reviewed-by: Murray Read Reviewed-by: Alex --- src/network/bearer/qnetworkconfigmanager.cpp | 9 ++++--- src/network/bearer/qnetworkconfigmanager_p.cpp | 36 ++++++++++++++++---------- src/network/bearer/qnetworkconfigmanager_p.h | 3 +++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/network/bearer/qnetworkconfigmanager.cpp b/src/network/bearer/qnetworkconfigmanager.cpp index e665811..2646f0c 100644 --- a/src/network/bearer/qnetworkconfigmanager.cpp +++ b/src/network/bearer/qnetworkconfigmanager.cpp @@ -60,8 +60,9 @@ Q_GLOBAL_STATIC(QMutex, connManager_mutex) static void connManager_cleanup() { // this is not atomic or thread-safe! - delete connManager_ptr.load(); - connManager_ptr.store(0); + QNetworkConfigurationManagerPrivate *cmp = connManager_ptr.fetchAndStoreAcquire(0); + if (cmp) + cmp->cleanup(); } void QNetworkConfigurationManagerPrivate::addPostRoutine() @@ -80,12 +81,12 @@ QNetworkConfigurationManagerPrivate *qNetworkConfigurationManagerPrivate() if (QCoreApplicationPrivate::mainThread() == QThread::currentThread()) { // right thread or no main thread yet ptr->addPostRoutine(); - ptr->updateConfigurations(); + ptr->initialize(); } else { // wrong thread, we need to make the main thread do this QObject *obj = new QObject; QObject::connect(obj, SIGNAL(destroyed()), ptr, SLOT(addPostRoutine()), Qt::DirectConnection); - ptr->updateConfigurations(); // this moves us to the main thread + ptr->initialize(); // this moves us to the right thread obj->moveToThread(QCoreApplicationPrivate::mainThread()); obj->deleteLater(); } diff --git a/src/network/bearer/qnetworkconfigmanager_p.cpp b/src/network/bearer/qnetworkconfigmanager_p.cpp index b203875..a9b0968 100644 --- a/src/network/bearer/qnetworkconfigmanager_p.cpp +++ b/src/network/bearer/qnetworkconfigmanager_p.cpp @@ -66,11 +66,31 @@ QNetworkConfigurationManagerPrivate::QNetworkConfigurationManagerPrivate() qRegisterMetaType("QNetworkConfigurationPrivatePointer"); } +void QNetworkConfigurationManagerPrivate::initialize() +{ + //Two stage construction, because we only want to do this heavyweight work for the winner of the Q_GLOBAL_STATIC race. + bearerThread = new QThread(); + bearerThread->moveToThread(QCoreApplicationPrivate::mainThread()); // because cleanup() is called in main thread context. + moveToThread(bearerThread); + bearerThread->start(); + updateConfigurations(); +} + QNetworkConfigurationManagerPrivate::~QNetworkConfigurationManagerPrivate() { QMutexLocker locker(&mutex); qDeleteAll(sessionEngines); + if (bearerThread) + bearerThread->quit(); +} + +void QNetworkConfigurationManagerPrivate::cleanup() +{ + QThread* thread = bearerThread; + deleteLater(); + if (thread->wait(5000)) + delete thread; } QNetworkConfiguration QNetworkConfigurationManagerPrivate::defaultConfiguration() const @@ -350,13 +370,6 @@ void QNetworkConfigurationManagerPrivate::updateConfigurations() if (qobject_cast(sender())) return; - if (thread() != QCoreApplicationPrivate::mainThread()) { - if (thread() != QThread::currentThread()) - return; - - moveToThread(QCoreApplicationPrivate::mainThread()); - } - updating = false; #ifndef QT_NO_LIBRARY @@ -375,7 +388,7 @@ void QNetworkConfigurationManagerPrivate::updateConfigurations() else sessionEngines.append(engine); - engine->moveToThread(QCoreApplicationPrivate::mainThread()); + engine->moveToThread(bearerThread); connect(engine, SIGNAL(updateCompleted()), this, SLOT(updateConfigurations())); @@ -411,14 +424,9 @@ void QNetworkConfigurationManagerPrivate::updateConfigurations() if (firstUpdate) { firstUpdate = false; QList enginesToInitialize = sessionEngines; //shallow copy the list in case it is modified when we unlock mutex - Qt::ConnectionType connectionType; - if (QCoreApplicationPrivate::mainThread() == QThread::currentThread()) - connectionType = Qt::DirectConnection; - else - connectionType = Qt::BlockingQueuedConnection; locker.unlock(); foreach (QBearerEngine* engine, enginesToInitialize) { - QMetaObject::invokeMethod(engine, "initialize", connectionType); + QMetaObject::invokeMethod(engine, "initialize", Qt::BlockingQueuedConnection); } } } diff --git a/src/network/bearer/qnetworkconfigmanager_p.h b/src/network/bearer/qnetworkconfigmanager_p.h index f96aedf..35b1f2b 100644 --- a/src/network/bearer/qnetworkconfigmanager_p.h +++ b/src/network/bearer/qnetworkconfigmanager_p.h @@ -89,6 +89,8 @@ public: void enablePolling(); void disablePolling(); + void initialize(); + void cleanup(); public Q_SLOTS: void updateConfigurations(); @@ -112,6 +114,7 @@ private Q_SLOTS: private: Q_INVOKABLE void startPolling(); QTimer *pollTimer; + QThread *bearerThread; private: mutable QMutex mutex; -- 2.7.4