From 2e12f6e63ddb1ea3c79660cd99ffb96d4cd0bd82 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 14 Apr 2015 17:02:48 -0700 Subject: [PATCH] Fix deadlock if the last reference is dropped during delivery We increase the reference count of the connection during delivery of an incoming message, so it's possible that the corresponding deref will drop the last reference to the connection: another thread may have called disconnectFromBus/Peer. However, during destruction we try to drain the incoming socket queue, so we need to acquire the dispatch lock again. The solution is to always use deleteLater(), which means the deleteYourself() function is unnecessary. Change-Id: I27eaacb532114dd188c4ffff13d507039fcf7b6a Reviewed-by: Albert Astals Cid Reviewed-by: Alex Blasche --- src/dbus/qdbusconnection.cpp | 6 +++--- src/dbus/qdbusconnection_p.h | 1 - src/dbus/qdbusintegrator.cpp | 14 -------------- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/dbus/qdbusconnection.cpp b/src/dbus/qdbusconnection.cpp index 0adb3dd748..fd8b34b27e 100644 --- a/src/dbus/qdbusconnection.cpp +++ b/src/dbus/qdbusconnection.cpp @@ -68,7 +68,7 @@ void QDBusConnectionManager::removeConnection(const QString &name) QDBusConnectionPrivate *d = 0; d = connectionHash.take(name); if (d && !d->ref.deref()) - d->deleteYourself(); + d->deleteLater(); // Static objects may be keeping the connection open. // However, it is harmless to have outstanding references to a connection that is @@ -268,7 +268,7 @@ QDBusConnection::QDBusConnection(QDBusConnectionPrivate *dd) QDBusConnection::~QDBusConnection() { if (d && !d->ref.deref()) - d->deleteYourself(); + d->deleteLater(); } /*! @@ -283,7 +283,7 @@ QDBusConnection &QDBusConnection::operator=(const QDBusConnection &other) if (other.d) other.d->ref.ref(); if (d && !d->ref.deref()) - d->deleteYourself(); + d->deleteLater(); d = other.d; return *this; } diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 752ca9c37a..8195ec5b3a 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -187,7 +187,6 @@ public: // public methods are entry points from other objects explicit QDBusConnectionPrivate(QObject *parent = 0); ~QDBusConnectionPrivate(); - void deleteYourself(); void setBusService(const QDBusConnection &connection); void setPeer(DBusConnection *connection, const QDBusErrorInternal &error); diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index 22c1a3e363..796f41bffc 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -1065,20 +1065,6 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate() server = 0; } -void QDBusConnectionPrivate::deleteYourself() -{ - if (thread() && thread() != QThread::currentThread()) { - // last reference dropped while not in the correct thread - // ask the correct thread to delete - - // note: since we're posting an event to another thread, we - // must consider deleteLater() to take effect immediately - deleteLater(); - } else { - delete this; - } -} - void QDBusConnectionPrivate::closeConnection() { QDBusWriteLocker locker(CloseConnectionAction, this);