From 68964b1023afa0eadee4ead5be8365b687eeec79 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 3 Apr 2015 22:46:52 -0700 Subject: [PATCH] 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 Reviewed-by: Alex Blasche --- src/dbus/qdbusconnection.cpp | 44 ++++++++++++++--------------- src/dbus/qdbusconnectionmanager_p.h | 4 +++ src/dbus/qdbusintegrator.cpp | 8 +++--- 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 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)