Remove some aggregation deadlocks.

You are not allowed to add components that belong to other aggregate
(including aggregates themselves). Warn in this case instead of locking
up.

Task-number: QTCREATORBUG-4926
Change-Id: I4908fb9019efbc2fa3b7c3c57e08cc1d7f8f3e2c
Reviewed-on: http://codereview.qt.nokia.com/679
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Bill King <bill.king@nokia.com>

diff --git a/src/libs/aggregation/aggregate.cpp b/src/libs/aggregation/aggregate.cpp
index 774752d..f6e4741 100644 (file)
--- a/src/libs/aggregation/aggregate.cpp
+++ b/src/libs/aggregation/aggregate.cpp
@@ -33,6 +33,7 @@
 #include "aggregate.h"
 
 #include <QtCore/QWriteLocker>
+#include <QtCore/QDebug>
 
 /*!
     \namespace Aggregation
@@ -228,6 +229,8 @@ void Aggregate::deleteSelf(QObject *obj)
     \fn void Aggregate::add(QObject *component)
 
     Adds the \a component to the aggregate.
+    You can't add a component that is part of a different aggregate
+    or an aggregate itself.
 
     \sa Aggregate::remove()
 */
@@ -240,8 +243,10 @@ void Aggregate::add(QObject *component)
         Aggregate *parentAggregation = aggregateMap().value(component);
         if (parentAggregation == this)
             return;
-        if (parentAggregation)
-            parentAggregation->remove(component);
+        if (parentAggregation) {
+            qWarning() << "Cannot add a component that belongs to a different aggregate" << component;
+            return;
+        }
         m_components.append(component);
         connect(component, SIGNAL(destroyed(QObject*)), this, SLOT(deleteSelf(QObject*)));
         aggregateMap().insert(component, this);

diff --git a/tests/auto/aggregation/tst_aggregate.cpp b/tests/auto/aggregation/tst_aggregate.cpp
index f5a99de..8126c63 100644 (file)
--- a/tests/auto/aggregation/tst_aggregate.cpp
+++ b/tests/auto/aggregation/tst_aggregate.cpp
@@ -185,6 +185,7 @@ void tst_Aggregate::queryAll()
 void tst_Aggregate::parentAggregate()
 {
     Aggregation::Aggregate aggregation;
+    Aggregation::Aggregate aggregation2;
     Interface1 *component1 = new Interface1;
     Interface11 *component11 = new Interface11;
     QObject *component2 = new QObject;
@@ -194,6 +195,15 @@ void tst_Aggregate::parentAggregate()
     QCOMPARE(Aggregation::Aggregate::parentAggregate(component1), &aggregation);
     QCOMPARE(Aggregation::Aggregate::parentAggregate(component11), &aggregation);
     QCOMPARE(Aggregation::Aggregate::parentAggregate(component2), (Aggregation::Aggregate *)0);
+    // test reparenting a component to another aggregate (should warn but not work)
+    aggregation2.add(component11);
+    QCOMPARE(Aggregation::Aggregate::parentAggregate(component11), &aggregation);
+    // test adding an aggregate to an aggregate (should warn but not work)
+    aggregation.add(&aggregation2);
+    QCOMPARE(Aggregation::Aggregate::parentAggregate(&aggregation2), &aggregation2);
+    // test removing an object from an aggregation.
+    aggregation.remove(component11);
+    QCOMPARE(Aggregation::Aggregate::parentAggregate(component11), (Aggregation::Aggregate *)0);
 }
 
 QTEST_MAIN(tst_Aggregate)