QNode: stop using hash<node,connection> for bookkeepingv5.14.0-beta3

It is totally valid to have actually the same node used for 2 distinct
connections (e.g setting 2 different node properties to the same node).
With the hash, the second setter call would overwrite the first connection
resulting in leaving a dangling connection around potentially resulting in
crashes.

Instead use a QVector<pair<node, connection>> and adjust code accordingly.

Change-Id: I49870c409c3f7b629c8f1bdfcb8757a904db2490
Reviewed-by: Mike Krus <mike.krus@kdab.com>

2 files changed, 17 insertions, 9 deletions

diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index 739b46cfe..af16eaa21 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -792,10 +792,9 @@ QNode::~QNode()
 {
     Q_D(QNode);
     // Disconnect each connection that was stored
-    for (auto it = d->m_destructionConnections.begin(), end = d->m_destructionConnections.end(); it != end; ++it)
-        QObject::disconnect(it.value());
+    for (const auto &nodeConnectionPair : qAsConst(d->m_destructionConnections))
+        QObject::disconnect(nodeConnectionPair.second);
     d->m_destructionConnections.clear();
-
     Q_EMIT nodeDestroyed();
 
     // Notify the backend that the parent lost this node as a child and
diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h
index a7a300a5e..208474802 100644
--- a/src/core/nodes/qnode_p.h
+++ b/src/core/nodes/qnode_p.h
@@ -124,7 +124,7 @@ public:
         // If the node is destoyed, we make sure not to keep a dangling pointer to it
         Q_Q(QNode);
         auto f = [q, func]() { (static_cast<Caller *>(q)->*func)(nullptr); };
-        m_destructionConnections.insert(node, QObject::connect(node, &QNode::nodeDestroyed, f));
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
     }
 
     template<typename Caller, typename NodeType>
@@ -133,7 +133,7 @@ public:
         // If the node is destoyed, we make sure not to keep a dangling pointer to it
         Q_Q(QNode);
         auto f = [q, func, node]() { (static_cast<Caller *>(q)->*func)(node); };
-        m_destructionConnections.insert(node, QObject::connect(node, &QNode::nodeDestroyed, f));
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
     }
 
     template<typename Caller, typename ValueType>
@@ -146,7 +146,7 @@ public:
         // If the node is destoyed, we make sure not to keep a dangling pointer to it
         Q_Q(QNode);
         auto f = [q, func, resetValue]() { (static_cast<Caller *>(q)->*func)(resetValue); };
-        m_destructionConnections.insert(node, QObject::connect(node, &QNode::nodeDestroyed, f));
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
     }
 
     template<typename Caller, typename NodeType>
@@ -154,12 +154,21 @@ public:
     {
         // If the node is destoyed, we make sure not to keep a dangling pointer to it
         auto f = [this, func, node]() { (static_cast<Caller *>(this)->*func)(node); };
-        m_destructionConnections.insert(node, QObject::connect(node, &QNode::nodeDestroyed, f));
+        m_destructionConnections.push_back({node, QObject::connect(node, &QNode::nodeDestroyed, f)});
     }
 
     void unregisterDestructionHelper(QNode *node)
     {
-        QObject::disconnect(m_destructionConnections.take(node));
+        m_destructionConnections.erase(std::remove_if(m_destructionConnections.begin(),
+                                                      m_destructionConnections.end(),
+                                                      [this, node] (const QPair<QNode *, QMetaObject::Connection> &nodeConnectionPair) {
+                                                          if (nodeConnectionPair.first == node) {
+                                                              QObject::disconnect(nodeConnectionPair.second);
+                                                              return true;
+                                                          }
+                                                          return false;
+                                                      }),
+                                       m_destructionConnections.end());
     }
 
     static const QMetaObject *findStaticMetaObject(const QMetaObject *metaObject);
@@ -184,7 +193,7 @@ private:
     friend class PropertyChangeHandler<QNodePrivate>;
     bool m_propertyChangesSetup;
     PropertyChangeHandler<QNodePrivate> m_signals;
-    QHash<QNode *, QMetaObject::Connection> m_destructionConnections;
+    QVector<QPair<QNode *, QMetaObject::Connection>> m_destructionConnections;
 };
 
 class NodePostConstructorInit : public QObject