DBus: fix deadlock when destroying QDBusServer

Observed infrequently in the QDBus tests, it would deadlock when
destroying QDBusServer at the same time as qDBusNewConnection was being
executed as they were locking the same locks, but in opposite order.

QDBusServer locks d->lock, then QDBusConnectionManager::instance()->mutex.
While qDBusNewConnection locks QDBusConnectionManager::instance()->mutex,
then serverConnection->lock (and serverConnection here
is QDBusServer's d-pointer).

QOrderedMutexLocker cannot be used in this situation because it
operates on QMutex*, which d->lock (QReadWriteLock) is not.

Change the code to lock QDBusConnectionManager's mutex before d->lock
and then unlock the QMutexLocker where it would previously destruct.
If QDBusConnectionManager has already been destroyed then we pass a
nullptr to the QMutexLocker which is fine and will not do anything.

Fixes: QTBUG-74635
Change-Id: I7f02d7759da67377996ef042c81b0969ccb8aadb
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

1 files changed, 6 insertions, 2 deletions

diff --git a/src/dbus/qdbusserver.cpp b/src/dbus/qdbusserver.cpp
index a2dfb86164..3ac5c794e1 100644
--- a/src/dbus/qdbusserver.cpp
+++ b/src/dbus/qdbusserver.cpp
@@ -109,12 +109,16 @@ QDBusServer::QDBusServer(QObject *parent)
 */
 QDBusServer::~QDBusServer()
 {
-    QWriteLocker locker(&d->lock);
+    QMutex *managerMutex = nullptr;
+    if (QDBusConnectionManager::instance())
+        managerMutex = &QDBusConnectionManager::instance()->mutex;
+    QMutexLocker locker(managerMutex);
+    QWriteLocker writeLocker(&d->lock);
     if (QDBusConnectionManager::instance()) {
-        QMutexLocker locker(&QDBusConnectionManager::instance()->mutex);
         for (const QString &name : qAsConst(d->serverConnectionNames))
             QDBusConnectionManager::instance()->removeConnection(name);
         d->serverConnectionNames.clear();
+        locker.unlock();
     }
     d->serverObject = nullptr;
     d->ref.store(0);