From ad66dbe305cff72443f4d3484191872d56e6dfbb Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Wed, 27 Apr 2016 22:34:26 -0700
Subject: [PATCH] Disconnect signals from each QObject only once in
QDBusConnectionPrivate
Because the moment we disconnect from the object's destroyed() signal,
it may get destroyed in another thread. If the same object appears more
than once in the object tree or in the signal hook table, we could be
accessing a dangling pointer.
Task-number: QTBUG-52988
Change-Id: Ifea6e497f11a461db432ffff14496f0f83889104
Reviewed-by: Weng Xuetian <wengxt@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
src/dbus/qdbusconnection_p.h | 2 +-
src/dbus/qdbusintegrator.cpp | 31 ++++++++++++++++++++-----------
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h
index b733a688563..fff9f29b031 100644
--- a/src/dbus/qdbusconnection_p.h
+++ b/src/dbus/qdbusconnection_p.h
@@ -254,7 +254,7 @@ private:
const QVector<int> &metaTypes, int slotIdx);
SignalHookHash::Iterator removeSignalHookNoLock(SignalHookHash::Iterator it);
- void disconnectObjectTree(ObjectTreeNode &node);
+ void collectAllObjects(ObjectTreeNode &node, QSet<QObject *> &set);
bool isServiceRegisteredByThread(const QString &serviceName);
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index d0468f4af0e..147966b9b09 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -1071,17 +1071,18 @@ QDBusConnectionPrivate::~QDBusConnectionPrivate()
}
}
-void QDBusConnectionPrivate::disconnectObjectTree(QDBusConnectionPrivate::ObjectTreeNode &haystack)
+void QDBusConnectionPrivate::collectAllObjects(QDBusConnectionPrivate::ObjectTreeNode &haystack,
+ QSet<QObject *> &set)
{
QDBusConnectionPrivate::ObjectTreeNode::DataList::Iterator it = haystack.children.begin();
while (it != haystack.children.end()) {
- disconnectObjectTree(*it);
+ collectAllObjects(*it, set);
it++;
}
if (haystack.obj)
- haystack.obj->disconnect(this);
+ set.insert(haystack.obj);
}
void QDBusConnectionPrivate::closeConnection()
@@ -1110,15 +1111,23 @@ void QDBusConnectionPrivate::closeConnection()
// Disconnect all signals from signal hooks and from the object tree to
// avoid QObject::destroyed being sent to dbus daemon thread which has
- // already quit.
- SignalHookHash::iterator sit = signalHooks.begin();
- while (sit != signalHooks.end()) {
- sit.value().obj->disconnect(this);
- sit++;
+ // already quit. We need to make sure we disconnect exactly once per
+ // object, because if we tried a second time, we might be hitting a
+ // dangling pointer.
+ QSet<QObject *> allObjects;
+ collectAllObjects(rootNode, allObjects);
+ SignalHookHash::const_iterator sit = signalHooks.constBegin();
+ while (sit != signalHooks.constEnd()) {
+ allObjects.insert(sit.value().obj);
+ ++sit;
+ }
+
+ // now disconnect ourselves
+ QSet<QObject *>::const_iterator oit = allObjects.constBegin();
+ while (oit != allObjects.constEnd()) {
+ (*oit)->disconnect(this);
+ ++oit;
}
-
- disconnectObjectTree(rootNode);
- rootNode.children.clear(); // free resources
}
void QDBusConnectionPrivate::handleDBusDisconnection()
--
GitLab