Merge two Q_GLOBAL_STATICS into another and solve memory leak

The two global statics for the session and system buses aren't necessary
if they can't outlive the global static for QDBusConnectionManager
anyway. So merge them there. The extra mutex is necessary because the
QDBusConnection::connectToBus function will lock the regular mutex.

This solves a potential memory leak at exit as a side-effect. Before
this change, the session and system QDBusConnection object got destroyed
in the main thread during global destruction, so it had to post an event
to the QDBusConnectionManager thread to finish the destruction of the
private. However, QCoreApplication is already gone by this point, so the
QEvent::DeferredDelete event never got delivered.

After this commit, there's no global static to destroy the
QDBusConnection (there is no QDBusConnection holding a reference), so
the object gets destroyed in QDBusConnectionManager::run()'s cleanup
code.

Change-Id: I9a75ad8521ae4e5cbbe5ffff13d1b967ee1a7a7e
Reviewed-by: Albert Astals Cid <aacid@kde.org>
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

3 files changed, 30 insertions, 26 deletions

diff --git a/src/dbus/qdbusconnection.cpp b/src/dbus/qdbusconnection.cpp
index a5d674bb5a..6f0473d9e4 100644
--- a/src/dbus/qdbusconnection.cpp
+++ b/src/dbus/qdbusconnection.cpp
@@ -76,6 +76,24 @@ struct QDBusConnectionManager::ConnectionRequestData
     QDBusConnectionPrivate *result;
 };
 
+QDBusConnectionPrivate *QDBusConnectionManager::busConnection(QDBusConnection::BusType type)
+{
+    Q_STATIC_ASSERT(int(QDBusConnection::SessionBus) + int(QDBusConnection::SystemBus) == 1);
+    Q_ASSERT(type == QDBusConnection::SessionBus || type == QDBusConnection::SystemBus);
+
+    if (!qdbus_loadLibDBus())
+        return 0;
+
+    QMutexLocker lock(&defaultBusMutex);
+    if (defaultBuses[type])
+        return defaultBuses[type];
+
+    QString name = QStringLiteral("qt_default_session_bus");
+    if (type == QDBusConnection::SystemBus)
+        name = QStringLiteral("qt_default_system_bus");
+    return defaultBuses[type] = connectToBus(type, name);
+}
+
 QDBusConnectionPrivate *QDBusConnectionManager::connection(const QString &name) const
 {
     return connectionHash.value(name, 0);
@@ -102,6 +120,8 @@ QDBusConnectionManager::QDBusConnectionManager()
     connect(this, &QDBusConnectionManager::serverRequested,
             this, &QDBusConnectionManager::createServer, Qt::BlockingQueuedConnection);
     moveToThread(this);         // ugly, don't do this in other projects
+
+    defaultBuses[0] = defaultBuses[1] = Q_NULLPTR;
     start();
 }
 
@@ -1109,26 +1129,6 @@ bool QDBusConnection::unregisterService(const QString &serviceName)
     return false;
 }
 
-static const char _q_sessionBusName[] = "qt_default_session_bus";
-static const char _q_systemBusName[] = "qt_default_system_bus";
-
-class QDBusDefaultConnection: public QDBusConnection
-{
-    const char *ownName;
-public:
-    inline QDBusDefaultConnection(BusType type, const char *name)
-        : QDBusConnection(connectToBus(type, QString::fromLatin1(name))), ownName(name)
-    { }
-
-    inline ~QDBusDefaultConnection()
-    { disconnectFromBus(QString::fromLatin1(ownName)); }
-};
-
-Q_GLOBAL_STATIC_WITH_ARGS(QDBusDefaultConnection, _q_sessionBus,
-                          (QDBusConnection::SessionBus, _q_sessionBusName))
-Q_GLOBAL_STATIC_WITH_ARGS(QDBusDefaultConnection, _q_systemBus,
-                          (QDBusConnection::SystemBus, _q_systemBusName))
-
 /*!
     \fn QDBusConnection QDBusConnection::sessionBus()
 
@@ -1138,7 +1138,7 @@ Q_GLOBAL_STATIC_WITH_ARGS(QDBusDefaultConnection, _q_systemBus,
 */
 QDBusConnection QDBusConnection::sessionBus()
 {
-    return *_q_sessionBus();
+    return QDBusConnection(_q_manager()->busConnection(SessionBus));
 }
 
 /*!
@@ -1150,7 +1150,7 @@ QDBusConnection QDBusConnection::sessionBus()
 */
 QDBusConnection QDBusConnection::systemBus()
 {
-    return *_q_systemBus();
+    return QDBusConnection(_q_manager()->busConnection(SystemBus));
 }
 
 #if QT_DEPRECATED_SINCE(5,5)
diff --git a/src/dbus/qdbusconnectionmanager_p.h b/src/dbus/qdbusconnectionmanager_p.h
index 4527c562f8..3f815fdcd7 100644
--- a/src/dbus/qdbusconnectionmanager_p.h
+++ b/src/dbus/qdbusconnectionmanager_p.h
@@ -63,6 +63,7 @@ public:
     ~QDBusConnectionManager();
     static QDBusConnectionManager* instance();
 
+    QDBusConnectionPrivate *busConnection(QDBusConnection::BusType type);
     QDBusConnectionPrivate *connection(const QString &name) const;
     void removeConnection(const QString &name);
     void setConnection(const QString &name, QDBusConnectionPrivate *c);
@@ -85,6 +86,9 @@ private:
 
     QHash<QString, QDBusConnectionPrivate *> connectionHash;
 
+    QMutex defaultBusMutex;
+    QDBusConnectionPrivate *defaultBuses[2];
+
     mutable QMutex senderMutex;
     QString senderName; // internal; will probably change
 };
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index 9b0a51107a..22598a2f2d 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1043,6 +1043,10 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
                  "Timer and socket errors will follow and the program will probably crash",
                  qPrintable(name));
 
+    closeConnection();
+    rootNode.children.clear();  // free resources
+    qDeleteAll(cachedMetaObjects);
+
     if (mode == ClientMode) {
         // the bus service object holds a reference back to us;
         // we need to destroy it before we finish destroying ourselves
@@ -1052,10 +1056,6 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
         delete obj;
     }
 
-    closeConnection();
-    rootNode.children.clear();  // free resources
-    qDeleteAll(cachedMetaObjects);
-
     if (server)
         q_dbus_server_unref(server);
     if (connection)