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>
This commit is contained in:
Thiago Macieira 2015-04-03 22:46:52 -07:00
parent ea01d7784a
commit 68964b1023
3 changed files with 30 additions and 26 deletions

View File

@ -76,6 +76,24 @@ struct QDBusConnectionManager::ConnectionRequestData
QDBusConnectionPrivate *result; 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 QDBusConnectionPrivate *QDBusConnectionManager::connection(const QString &name) const
{ {
return connectionHash.value(name, 0); return connectionHash.value(name, 0);
@ -102,6 +120,8 @@ QDBusConnectionManager::QDBusConnectionManager()
connect(this, &QDBusConnectionManager::serverRequested, connect(this, &QDBusConnectionManager::serverRequested,
this, &QDBusConnectionManager::createServer, Qt::BlockingQueuedConnection); this, &QDBusConnectionManager::createServer, Qt::BlockingQueuedConnection);
moveToThread(this); // ugly, don't do this in other projects moveToThread(this); // ugly, don't do this in other projects
defaultBuses[0] = defaultBuses[1] = Q_NULLPTR;
start(); start();
} }
@ -1109,26 +1129,6 @@ bool QDBusConnection::unregisterService(const QString &serviceName)
return false; 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() \fn QDBusConnection QDBusConnection::sessionBus()
@ -1138,7 +1138,7 @@ Q_GLOBAL_STATIC_WITH_ARGS(QDBusDefaultConnection, _q_systemBus,
*/ */
QDBusConnection QDBusConnection::sessionBus() QDBusConnection QDBusConnection::sessionBus()
{ {
return *_q_sessionBus(); return QDBusConnection(_q_manager()->busConnection(SessionBus));
} }
/*! /*!
@ -1150,7 +1150,7 @@ QDBusConnection QDBusConnection::sessionBus()
*/ */
QDBusConnection QDBusConnection::systemBus() QDBusConnection QDBusConnection::systemBus()
{ {
return *_q_systemBus(); return QDBusConnection(_q_manager()->busConnection(SystemBus));
} }
#if QT_DEPRECATED_SINCE(5,5) #if QT_DEPRECATED_SINCE(5,5)

View File

@ -63,6 +63,7 @@ public:
~QDBusConnectionManager(); ~QDBusConnectionManager();
static QDBusConnectionManager* instance(); static QDBusConnectionManager* instance();
QDBusConnectionPrivate *busConnection(QDBusConnection::BusType type);
QDBusConnectionPrivate *connection(const QString &name) const; QDBusConnectionPrivate *connection(const QString &name) const;
void removeConnection(const QString &name); void removeConnection(const QString &name);
void setConnection(const QString &name, QDBusConnectionPrivate *c); void setConnection(const QString &name, QDBusConnectionPrivate *c);
@ -85,6 +86,9 @@ private:
QHash<QString, QDBusConnectionPrivate *> connectionHash; QHash<QString, QDBusConnectionPrivate *> connectionHash;
QMutex defaultBusMutex;
QDBusConnectionPrivate *defaultBuses[2];
mutable QMutex senderMutex; mutable QMutex senderMutex;
QString senderName; // internal; will probably change QString senderName; // internal; will probably change
}; };

View File

@ -1043,6 +1043,10 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
"Timer and socket errors will follow and the program will probably crash", "Timer and socket errors will follow and the program will probably crash",
qPrintable(name)); qPrintable(name));
closeConnection();
rootNode.children.clear(); // free resources
qDeleteAll(cachedMetaObjects);
if (mode == ClientMode) { if (mode == ClientMode) {
// the bus service object holds a reference back to us; // the bus service object holds a reference back to us;
// we need to destroy it before we finish destroying ourselves // we need to destroy it before we finish destroying ourselves
@ -1052,10 +1056,6 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
delete obj; delete obj;
} }
closeConnection();
rootNode.children.clear(); // free resources
qDeleteAll(cachedMetaObjects);
if (server) if (server)
q_dbus_server_unref(server); q_dbus_server_unref(server);
if (connection) if (connection)