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: 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/qdbusintegrator.cpp | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

(limited to 'src/dbus/qdbusintegrator.cpp')

diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index d0468f4af0..147966b9b0 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()
-- 
cgit v1.2.3