Don't crash if Connections::target is changed by one of its signal handlers

Reviewed-by: Michael Brasser

5 files changed, 67 insertions, 5 deletions

diff --git a/src/declarative/qml/qdeclarativeboundsignal.cpp b/src/declarative/qml/qdeclarativeboundsignal.cpp
index 00a93cc..eb352a2 100644
--- a/src/declarative/qml/qdeclarativeboundsignal.cpp
+++ b/src/declarative/qml/qdeclarativeboundsignal.cpp
@@ -95,7 +95,7 @@ QDeclarativeAbstractBoundSignal::~QDeclarativeAbstractBoundSignal()
 
 QDeclarativeBoundSignal::QDeclarativeBoundSignal(QObject *scope, const QMetaMethod &signal, 
                                QObject *parent)
-: m_expression(0), m_signal(signal), m_paramsValid(false), m_params(0)
+: m_expression(0), m_signal(signal), m_paramsValid(false), m_isEvaluating(false), m_params(0)
 {
     // A cached evaluation of the QDeclarativeExpression::value() slot index.
     //
@@ -111,7 +111,7 @@ QDeclarativeBoundSignal::QDeclarativeBoundSignal(QObject *scope, const QMetaMeth
 QDeclarativeBoundSignal::QDeclarativeBoundSignal(QDeclarativeContext *ctxt, const QString &val, 
                                QObject *scope, const QMetaMethod &signal,
                                QObject *parent)
-: m_expression(0), m_signal(signal), m_paramsValid(false), m_params(0)
+: m_expression(0), m_signal(signal), m_paramsValid(false), m_isEvaluating(false), m_params(0)
 {
     // A cached evaluation of the QDeclarativeExpression::value() slot index.
     //
@@ -169,6 +169,7 @@ QDeclarativeBoundSignal *QDeclarativeBoundSignal::cast(QObject *o)
 int QDeclarativeBoundSignal::qt_metacall(QMetaObject::Call c, int id, void **a)
 {
     if (c == QMetaObject::InvokeMetaMethod && id == evaluateIdx) {
+        m_isEvaluating = true;
         if (!m_paramsValid) {
             if (!m_signal.parameterTypes().isEmpty())
                 m_params = new QDeclarativeBoundSignalParameters(m_signal, this);
@@ -182,6 +183,7 @@ int QDeclarativeBoundSignal::qt_metacall(QMetaObject::Call c, int id, void **a)
                 QDeclarativeEnginePrivate::warning(m_expression->engine(), m_expression->error());
         }
         if (m_params) m_params->clearValues();
+        m_isEvaluating = false;
         return -1;
     } else {
         return QObject::qt_metacall(c, id, a);
diff --git a/src/declarative/qml/qdeclarativeboundsignal_p.h b/src/declarative/qml/qdeclarativeboundsignal_p.h
index bba4eec..06900d7 100644
--- a/src/declarative/qml/qdeclarativeboundsignal_p.h
+++ b/src/declarative/qml/qdeclarativeboundsignal_p.h
@@ -83,6 +83,8 @@ public:
     QDeclarativeExpression *expression() const;
     QDeclarativeExpression *setExpression(QDeclarativeExpression *);
 
+    bool isEvaluating() const { return m_isEvaluating; }
+
     static QDeclarativeBoundSignal *cast(QObject *);
 
 protected:
@@ -91,7 +93,8 @@ protected:
 private:
     QDeclarativeExpression *m_expression;
     QMetaMethod m_signal;
-    bool m_paramsValid;
+    bool m_paramsValid : 1;
+    bool m_isEvaluating : 1;
     QDeclarativeBoundSignalParameters *m_params;
 };
 
diff --git a/src/declarative/util/qdeclarativeconnections.cpp b/src/declarative/util/qdeclarativeconnections.cpp
index 596b306..c188521 100644
--- a/src/declarative/util/qdeclarativeconnections.cpp
+++ b/src/declarative/util/qdeclarativeconnections.cpp
@@ -150,8 +150,14 @@ void QDeclarativeConnections::setTarget(QObject *obj)
     Q_D(QDeclarativeConnections);
     if (d->target == obj)
         return;
-    foreach (QDeclarativeBoundSignal *s, d->boundsignals)
-        delete s;
+    foreach (QDeclarativeBoundSignal *s, d->boundsignals) {
+        // It is possible that target is being changed due to one of our signal
+        // handlers -> use deleteLater().
+        if (s->isEvaluating())
+            s->deleteLater();
+        else
+            delete s;
+    }
     d->boundsignals.clear();
     d->target = obj;
     connectSignals();
diff --git a/tests/auto/declarative/qdeclarativeconnection/data/connection-targetchange.qml b/tests/auto/declarative/qdeclarativeconnection/data/connection-targetchange.qml
new file mode 100644
index 0000000..bb9a3bc
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeconnection/data/connection-targetchange.qml
@@ -0,0 +1,25 @@
+import Qt 4.7
+
+Item {
+    Component {
+        id: item1
+        Item {
+            objectName: "item1"
+        }
+    }
+    Component {
+        id: item2
+        Item {
+            objectName: "item2"
+        }
+    }
+    Loader {
+        id: loader
+        sourceComponent: item1
+    }
+    Connections {
+        objectName: "connections"
+        target: loader.item
+        onWidthChanged: loader.sourceComponent = item2
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativeconnection/tst_qdeclarativeconnection.cpp b/tests/auto/declarative/qdeclarativeconnection/tst_qdeclarativeconnection.cpp
index f4914e1..0efae3b 100644
--- a/tests/auto/declarative/qdeclarativeconnection/tst_qdeclarativeconnection.cpp
+++ b/tests/auto/declarative/qdeclarativeconnection/tst_qdeclarativeconnection.cpp
@@ -58,6 +58,7 @@ private slots:
     void properties();
     void connection();
     void trimming();
+    void targetChanged();
 
 private:
     QDeclarativeEngine engine;
@@ -130,6 +131,31 @@ void tst_qdeclarativeconnection::trimming()
     delete item;
 }
 
+// Confirm that target can be changed by one of our signal handlers
+void tst_qdeclarativeconnection::targetChanged()
+{
+    QDeclarativeEngine engine;
+    QDeclarativeComponent c(&engine, QUrl::fromLocalFile(SRCDIR "/data/connection-targetchange.qml"));
+    QDeclarativeItem *item = qobject_cast<QDeclarativeItem*>(c.create());
+    QVERIFY(item != 0);
+
+    QDeclarativeConnections *connections = item->findChild<QDeclarativeConnections*>("connections");
+    QVERIFY(connections);
+
+    QDeclarativeItem *item1 = item->findChild<QDeclarativeItem*>("item1");
+    QVERIFY(item1);
+
+    item1->setWidth(200);
+
+    QDeclarativeItem *item2 = item->findChild<QDeclarativeItem*>("item2");
+    QVERIFY(item2);
+    QVERIFY(connections->target() == item2);
+
+    // If we don't crash then we're OK
+
+    delete item;
+}
+
 QTEST_MAIN(tst_qdeclarativeconnection)
 
 #include "tst_qdeclarativeconnection.moc"