More robust tracking of signal handler expression ownership.

Reference count the expressions, and improve testing.

Change-Id: I810509eae1c7608b367e9ff5f7891a294667a692
Reviewed-by: Chris Adams <christopher.adams@nokia.com>

13 files changed, 263 insertions, 82 deletions

diff --git a/src/qml/debugger/qqmlenginedebugservice.cpp b/src/qml/debugger/qqmlenginedebugservice.cpp
index f837da578f..b095e8f9bb 100644
--- a/src/qml/debugger/qqmlenginedebugservice.cpp
+++ b/src/qml/debugger/qqmlenginedebugservice.cpp
@@ -580,7 +580,7 @@ bool QQmlEngineDebugService::setBinding(int objectId,
                 } else if (hasValidSignal(object, propertyName)) {
                     QQmlBoundSignalExpression *qmlExpression = new QQmlBoundSignalExpression(QQmlContextData::get(context), object, expression.toString(),
                                                                                              false, filename, line, column);
-                    QQmlPropertyPrivate::setSignalExpression(property, qmlExpression);
+                    QQmlPropertyPrivate::takeSignalExpression(property, qmlExpression);
                 } else if (property.isProperty()) {
                     QQmlBinding *binding = new QQmlBinding(expression.toString(), false, object, QQmlContextData::get(context), filename, line, column);;
                     binding->setTarget(property);
diff --git a/src/qml/qml/qml.pri b/src/qml/qml/qml.pri
index fadfc9d833..1237740f31 100644
--- a/src/qml/qml/qml.pri
+++ b/src/qml/qml/qml.pri
@@ -72,6 +72,7 @@ HEADERS += \
     $$PWD/qqmlpropertyvaluesource.h \
     $$PWD/qqmlpropertyvalueinterceptor_p.h \
     $$PWD/qqmlboundsignal_p.h \
+    $$PWD/qqmlboundsignalexpressionpointer_p.h \
     $$PWD/qqmlparserstatus.h \
     $$PWD/qqmlproxymetaobject_p.h \
     $$PWD/qqmlvme_p.h \
diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp
index c0475b1ed9..b8b74ac2e9 100644
--- a/src/qml/qml/qqmlboundsignal.cpp
+++ b/src/qml/qml/qqmlboundsignal.cpp
@@ -233,7 +233,6 @@ QQmlBoundSignal::QQmlBoundSignal(QObject *scope, const QMetaMethod &signal,
 
 QQmlBoundSignal::~QQmlBoundSignal()
 {
-    delete m_expression;
     m_expression = 0;
     delete m_params;
 }
@@ -255,17 +254,32 @@ QQmlBoundSignalExpression *QQmlBoundSignal::expression() const
     Sets the signal expression to \a e.  Returns the current signal expression,
     or null if there is no signal expression.
 
-    The QQmlBoundSignal instance takes ownership of \a e.  The caller is 
-    assumes ownership of the returned QQmlExpression.
+    The QQmlBoundSignal instance adds a reference to \a e.  The caller
+    assumes ownership of the returned QQmlBoundSignalExpression reference.
 */
-QQmlBoundSignalExpression *QQmlBoundSignal::setExpression(QQmlBoundSignalExpression *e)
+QQmlBoundSignalExpressionPointer QQmlBoundSignal::setExpression(QQmlBoundSignalExpression *e)
 {
-    QQmlBoundSignalExpression *rv = m_expression;
+    QQmlBoundSignalExpressionPointer rv = m_expression;
     m_expression = e;
     if (m_expression) m_expression->setNotifyOnValueChanged(false);
     return rv;
 }
 
+/*!
+    Sets the signal expression to \a e.  Returns the current signal expression,
+    or null if there is no signal expression.
+
+    The QQmlBoundSignal instance takes ownership of \a e (and does not add a reference).  The caller
+    assumes ownership of the returned QQmlBoundSignalExpression reference.
+*/
+QQmlBoundSignalExpressionPointer QQmlBoundSignal::takeExpression(QQmlBoundSignalExpression *e)
+{
+    QQmlBoundSignalExpressionPointer rv = m_expression;
+    m_expression.take(e);
+    if (m_expression) m_expression->setNotifyOnValueChanged(false);
+    return rv;
+}
+
 void QQmlBoundSignal::subscriptionCallback(QQmlNotifierEndpoint *e, void **a)
 {
     QQmlBoundSignal *s = static_cast<QQmlBoundSignal*>(e);
@@ -401,6 +415,52 @@ int QQmlBoundSignalParameters::metaCall(QMetaObject::Call c, int id, void **a)
     }
 }
 
+////////////////////////////////////////////////////////////////////////
+
+QQmlBoundSignalExpressionPointer::QQmlBoundSignalExpressionPointer(QQmlBoundSignalExpression *o)
+: o(o)
+{
+    if (o) o->addref();
+}
+
+QQmlBoundSignalExpressionPointer::QQmlBoundSignalExpressionPointer(const QQmlBoundSignalExpressionPointer &other)
+: o(other.o)
+{
+    if (o) o->addref();
+}
+
+QQmlBoundSignalExpressionPointer::~QQmlBoundSignalExpressionPointer()
+{
+    if (o) o->release();
+}
+
+QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::operator=(const QQmlBoundSignalExpressionPointer &other)
+{
+    if (other.o) other.o->addref();
+    if (o) o->release();
+    o = other.o;
+    return *this;
+}
+
+QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::operator=(QQmlBoundSignalExpression *other)
+{
+    if (other) other->addref();
+    if (o) o->release();
+    o = other;
+    return *this;
+}
+
+/*!
+Takes ownership of \a other.  take() does *not* add a reference, as it assumes ownership
+of the callers reference of other.
+*/
+QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::take(QQmlBoundSignalExpression *other)
+{
+    if (o) o->release();
+    o = other;
+    return *this;
+}
+
 QT_END_NAMESPACE
 
 #include <qqmlboundsignal.moc>
diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h
index 7ce45aa646..3fb20a1290 100644
--- a/src/qml/qml/qqmlboundsignal_p.h
+++ b/src/qml/qml/qqmlboundsignal_p.h
@@ -57,20 +57,21 @@
 
 #include <private/qqmlabstractexpression_p.h>
 #include <private/qqmljavascriptexpression_p.h>
+#include <private/qqmlboundsignalexpressionpointer_p.h>
 #include <private/qqmlnotifier_p.h>
 #include <private/qflagpointer_p.h>
+#include <private/qqmlrefcount_p.h>
 #include <private/qobject_p.h>
 
 QT_BEGIN_NAMESPACE
 
-class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression : public QQmlAbstractExpression, public QQmlJavaScriptExpression
+class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression : public QQmlAbstractExpression, public QQmlJavaScriptExpression, public QQmlRefCount
 {
 public:
     QQmlBoundSignalExpression(QQmlContextData *ctxt, QObject *scope, const QByteArray &expression,
                               bool isRewritten, const QString &fileName, int line, int column);
     QQmlBoundSignalExpression(QQmlContextData *ctxt, QObject *scope, const QString &expression,
                               bool isRewritten, const QString &fileName, int line, int column);
-    ~QQmlBoundSignalExpression();
 
     // "inherited" from QQmlJavaScriptExpression.
     static QString expressionIdentifier(QQmlJavaScriptExpression *);
@@ -87,6 +88,8 @@ public:
     QQmlEngine *engine() const { return context() ? context()->engine : 0; }
 
 private:
+    ~QQmlBoundSignalExpression();
+
     v8::Persistent<v8::Object> m_v8qmlscope;
     v8::Persistent<v8::Function> m_v8function;
 
@@ -108,7 +111,8 @@ public:
 
     virtual int index() const = 0;
     virtual QQmlBoundSignalExpression *expression() const = 0;
-    virtual QQmlBoundSignalExpression *setExpression(QQmlBoundSignalExpression *) = 0;
+    virtual QQmlBoundSignalExpressionPointer setExpression(QQmlBoundSignalExpression *) = 0;
+    virtual QQmlBoundSignalExpressionPointer takeExpression(QQmlBoundSignalExpression *) = 0;
     virtual QObject *scope() = 0;
 
     void removeFromObject();
@@ -134,7 +138,8 @@ public:
     int index() const;
 
     QQmlBoundSignalExpression *expression() const;
-    QQmlBoundSignalExpression *setExpression(QQmlBoundSignalExpression *);
+    QQmlBoundSignalExpressionPointer setExpression(QQmlBoundSignalExpression *);
+    QQmlBoundSignalExpressionPointer takeExpression(QQmlBoundSignalExpression *);
     QObject *scope() { return *m_scope; }
 
     static void subscriptionCallback(QQmlNotifierEndpoint *e, void **);
@@ -142,7 +147,7 @@ public:
     bool isEvaluating() const { return m_scope.flag(); }
 
 private:
-    QQmlBoundSignalExpression *m_expression;
+    QQmlBoundSignalExpressionPointer m_expression;
     QQmlBoundSignalParameters *m_params;
     // We store some flag bits in the following flag pointer.
     //    m_scope:flag1 - m_isEvaluating
diff --git a/src/qml/qml/qqmlboundsignalexpressionpointer_p.h b/src/qml/qml/qqmlboundsignalexpressionpointer_p.h
new file mode 100644
index 0000000000..cc2106f496
--- /dev/null
+++ b/src/qml/qml/qqmlboundsignalexpressionpointer_p.h
@@ -0,0 +1,83 @@
+/****************************************************************************
+**
+** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/
+**
+** This file is part of the QtQml module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** GNU Lesser General Public License Usage
+** This file may be used under the terms of the GNU Lesser General Public
+** License version 2.1 as published by the Free Software Foundation and
+** appearing in the file LICENSE.LGPL included in the packaging of this
+** file. Please review the following information to ensure the GNU Lesser
+** General Public License version 2.1 requirements will be met:
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Nokia gives you certain additional
+** rights. These rights are described in the Nokia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU General
+** Public License version 3.0 as published by the Free Software Foundation
+** and appearing in the file LICENSE.GPL included in the packaging of this
+** file. Please review the following information to ensure the GNU General
+** Public License version 3.0 requirements will be met:
+** http://www.gnu.org/copyleft/gpl.html.
+**
+** Other Usage
+** Alternatively, this file may be used in accordance with the terms and
+** conditions contained in a signed written agreement between you and Nokia.
+**
+**
+**
+**
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H
+#define QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists purely as an
+// implementation detail.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+#include <QtQml/qtqmlglobal.h>
+
+QT_BEGIN_NAMESPACE
+
+class QQmlBoundSignalExpression;
+class Q_QML_EXPORT QQmlBoundSignalExpressionPointer
+{
+public:
+    inline QQmlBoundSignalExpressionPointer() : o(0) {}
+    QQmlBoundSignalExpressionPointer(QQmlBoundSignalExpression *);
+    QQmlBoundSignalExpressionPointer(const QQmlBoundSignalExpressionPointer &);
+    ~QQmlBoundSignalExpressionPointer();
+
+    QQmlBoundSignalExpressionPointer &operator=(const QQmlBoundSignalExpressionPointer &o);
+    QQmlBoundSignalExpressionPointer &operator=(QQmlBoundSignalExpression *);
+
+    inline QQmlBoundSignalExpression* operator->() const { return o; }
+    inline QQmlBoundSignalExpression& operator*() const { return *o; }
+    inline operator QQmlBoundSignalExpression*() const { return o; }
+
+    QQmlBoundSignalExpressionPointer &take(QQmlBoundSignalExpression *);
+
+private:
+    QQmlBoundSignalExpression *o;
+};
+
+QT_END_NAMESPACE
+
+#endif // QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H
diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 002986157f..d791d73cbf 100644
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -943,17 +943,34 @@ QQmlPropertyPrivate::signalExpression(const QQmlProperty &that)
 
 /*!
     Set the signal expression associated with this signal property to \a expr.
-    Returns the existing signal expression (if any), otherwise 0.
+    Returns the existing signal expression (if any), otherwise null.
 
-    Ownership of \a expr transfers to QML.  Ownership of the return value is
-    assumed by the caller.
+    A reference to \a expr will be added by QML.  Ownership of the return value
+    reference is assumed by the caller.
 */
-QQmlBoundSignalExpression *
+QQmlBoundSignalExpressionPointer
 QQmlPropertyPrivate::setSignalExpression(const QQmlProperty &that,
                                          QQmlBoundSignalExpression *expr)
 {
+    if (expr)
+        expr->addref();
+    return QQmlPropertyPrivate::takeSignalExpression(that, expr);
+}
+
+/*!
+    Set the signal expression associated with this signal property to \a expr.
+    Returns the existing signal expression (if any), otherwise null.
+
+    Ownership of \a expr transfers to QML.  Ownership of the return value
+    reference is assumed by the caller.
+*/
+QQmlBoundSignalExpressionPointer
+QQmlPropertyPrivate::takeSignalExpression(const QQmlProperty &that,
+                                         QQmlBoundSignalExpression *expr)
+{
     if (!(that.type() & QQmlProperty::SignalProperty)) {
-        delete expr;
+        if (expr)
+            expr->release();
         return 0;
     }
 
@@ -967,15 +984,13 @@ QQmlPropertyPrivate::setSignalExpression(const QQmlProperty &that,
         signalHandler = signalHandler->m_nextSignal;
 
     if (signalHandler)
-        return signalHandler->setExpression(expr);
+        return signalHandler->takeExpression(expr);
 
     if (expr) {
         QQmlBoundSignal *signal = new QQmlBoundSignal(that.d->object, that.method(), that.d->object);
-        QQmlBoundSignalExpression *oldExpr = signal->setExpression(expr);
-        return oldExpr;
-    } else {
-        return 0;
+        signal->takeExpression(expr);
     }
+    return 0;
 }
 
 /*!
diff --git a/src/qml/qml/qqmlproperty_p.h b/src/qml/qml/qqmlproperty_p.h
index e33c95ae41..cba7849ea0 100644
--- a/src/qml/qml/qqmlproperty_p.h
+++ b/src/qml/qml/qqmlproperty_p.h
@@ -60,11 +60,11 @@
 #include <private/qtqmlglobal_p.h>
 #include <private/qqmlpropertycache_p.h>
 #include <private/qqmlguard_p.h>
+#include <private/qqmlboundsignalexpressionpointer_p.h>
 
 QT_BEGIN_NAMESPACE
 
 class QQmlContext;
-class QQmlBoundSignalExpression;
 class QQmlEnginePrivate;
 class QQmlJavaScriptExpression;
 class Q_QML_PRIVATE_EXPORT QQmlPropertyPrivate : public QQmlRefCount
@@ -141,8 +141,10 @@ public:
                                                    QQmlAbstractBinding *,
                                                    WriteFlags flags = DontRemoveBinding);
     static QQmlBoundSignalExpression *signalExpression(const QQmlProperty &that);
-    static QQmlBoundSignalExpression *setSignalExpression(const QQmlProperty &that,
-                                                          QQmlBoundSignalExpression *) ;
+    static QQmlBoundSignalExpressionPointer setSignalExpression(const QQmlProperty &that,
+                                                                QQmlBoundSignalExpression *);
+    static QQmlBoundSignalExpressionPointer takeSignalExpression(const QQmlProperty &that,
+                                                                 QQmlBoundSignalExpression *);
     static bool write(const QQmlProperty &that, const QVariant &, WriteFlags);
     static bool writeBinding(QObject *, const QQmlPropertyData &,
                              QQmlContextData *context,
diff --git a/src/qml/qml/qqmlvme.cpp b/src/qml/qml/qqmlvme.cpp
index 47ea690973..6047688a95 100644
--- a/src/qml/qml/qqmlvme.cpp
+++ b/src/qml/qml/qqmlvme.cpp
@@ -739,7 +739,7 @@ QObject *QQmlVME::run(QList<QQmlError> *errors,
             QQmlBoundSignal *bs = new QQmlBoundSignal(target, signal, target);
             QQmlBoundSignalExpression *expr =
                 new QQmlBoundSignalExpression(CTXT, context, DATAS.at(instr.value), true, COMP->name, instr.line, instr.column);
-            bs->setExpression(expr);
+            bs->takeExpression(expr);
         QML_END_INSTR(StoreSignal)
 
         QML_BEGIN_INSTR(StoreImportedScript)
diff --git a/src/quick/util/qquickconnections.cpp b/src/quick/util/qquickconnections.cpp
index 8116ac3425..1740224fad 100644
--- a/src/quick/util/qquickconnections.cpp
+++ b/src/quick/util/qquickconnections.cpp
@@ -292,7 +292,7 @@ void QQuickConnections::connectSignals()
 
             QQmlBoundSignalExpression *expression = ctxtdata ?
                 new QQmlBoundSignalExpression(ctxtdata, 0, script, true, location, line, column) : 0;
-            signal->setExpression(expression);
+            signal->takeExpression(expression);
             d->boundsignals += signal;
         } else {
             if (!d->ignoreUnknownSignals)
diff --git a/src/quick/util/qquickpropertychanges.cpp b/src/quick/util/qquickpropertychanges.cpp
index 49df009af2..e2486d3a6c 100644
--- a/src/quick/util/qquickpropertychanges.cpp
+++ b/src/quick/util/qquickpropertychanges.cpp
@@ -139,44 +139,23 @@ QT_BEGIN_NAMESPACE
 class QQuickReplaceSignalHandler : public QQuickActionEvent
 {
 public:
-    QQuickReplaceSignalHandler() : expression(0), reverseExpression(0),
-                                rewindExpression(0), ownedExpression(0), ownedExpressionWatcher(0) {}
-    ~QQuickReplaceSignalHandler() {
-        delete ownedExpression;
-    }
+    QQuickReplaceSignalHandler() {}
+    ~QQuickReplaceSignalHandler() {}
 
     virtual EventType type() const { return SignalHandler; }
 
     QQmlProperty property;
-    QQmlBoundSignalExpression *expression;
-    QQmlBoundSignalExpression *reverseExpression;
-    QQmlBoundSignalExpression *rewindExpression;
-    QQmlBoundSignalExpression *ownedExpression;
-    QQmlAbstractExpression::DeleteWatcher *ownedExpressionWatcher; // TODO: refactor the ownership impl.
+    QQmlBoundSignalExpressionPointer expression;
+    QQmlBoundSignalExpressionPointer reverseExpression;
+    QQmlBoundSignalExpressionPointer rewindExpression;
 
     virtual void execute(Reason) {
-        ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, expression);
-        if (ownedExpression == expression) {
-            delete ownedExpressionWatcher;
-            ownedExpressionWatcher = 0;
-            ownedExpression = 0;
-        } else if (ownedExpression) {
-            delete ownedExpressionWatcher;
-            ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression);
-        }
+        QQmlPropertyPrivate::setSignalExpression(property, expression);
     }
 
     virtual bool isReversable() { return true; }
     virtual void reverse(Reason) {
-        ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, reverseExpression);
-        if (ownedExpression == reverseExpression) {
-            delete ownedExpressionWatcher;
-            ownedExpressionWatcher = 0;
-            ownedExpression = 0;
-        } else if (ownedExpression) {
-            delete ownedExpressionWatcher;
-            ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression);
-        }
+        QQmlPropertyPrivate::setSignalExpression(property, reverseExpression);
     }
 
     virtual void saveOriginals() {
@@ -192,18 +171,10 @@ public:
         if (rsh == this)
             return;
         reverseExpression = rsh->reverseExpression;
-        if (rsh->ownedExpression == reverseExpression) {
-            ownedExpression = rsh->ownedExpression;
-            rsh->ownedExpression = 0;
-            delete ownedExpressionWatcher;
-            ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression);
-        }
     }
 
     virtual void rewind() {
-        ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, rewindExpression);
-        if (ownedExpression == rewindExpression)
-            ownedExpression = 0;
+        QQmlPropertyPrivate::setSignalExpression(property, rewindExpression);
     }
     virtual void saveCurrentValues() {
         rewindExpression = QQmlPropertyPrivate::signalExpression(property);
@@ -370,7 +341,7 @@ void QQuickPropertyChangesPrivate::decode()
 
             QQuickReplaceSignalHandler *handler = new QQuickReplaceSignalHandler;
             handler->property = prop;
-            handler->expression = new QQmlBoundSignalExpression(QQmlContextData::get(qmlContext(q)), object, expression, false, url.toString(), line, column);
+            handler->expression.take(new QQmlBoundSignalExpression(QQmlContextData::get(qmlContext(q)), object, expression, false, url.toString(), line, column));
             signalReplacements << handler;
         } else if (isScript) { // binding
             QString expression = data.toString();
@@ -390,7 +361,6 @@ void QQuickPropertyChangesPrivate::decode()
             properties << qMakePair(name, data);
         }
     }
-
     decoded = true;
     data.clear();
 }
diff --git a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
index d99ed303ba..a65693bb8f 100644
--- a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
+++ b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp
@@ -181,7 +181,7 @@ void tst_qqmlproperty::qmlmetaproperty()
     QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
     QVERIFY(binding == 0);
     QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-    QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+    QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
     QVERIFY(sigExprWatcher.wasDeleted());
     QCOMPARE(prop.index(), -1);
     QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -285,7 +285,7 @@ void tst_qqmlproperty::qmlmetaproperty_object()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), -1);
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -335,7 +335,7 @@ void tst_qqmlproperty::qmlmetaproperty_object()
         QVERIFY(binding != 0);
         QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty"));
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -388,7 +388,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), -1);
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -438,7 +438,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string()
         QVERIFY(binding != 0);
         QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty"));
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -486,7 +486,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(!sigExprWatcher.wasDeleted());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr);
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("clicked()"));
@@ -535,7 +535,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(!sigExprWatcher.wasDeleted());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr);
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("oddlyNamedNotifySignal()"));
@@ -589,7 +589,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_context()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), -1);
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -639,7 +639,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_context()
         QVERIFY(binding != 0);
         QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty"));
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -692,7 +692,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), -1);
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -742,7 +742,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context()
         QVERIFY(binding != 0);
         QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(sigExprWatcher.wasDeleted());
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty"));
         QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1);
@@ -790,7 +790,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(!sigExprWatcher.wasDeleted());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr);
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("clicked()"));
@@ -839,7 +839,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context()
         QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0);
         QVERIFY(binding == 0);
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0);
-        QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0);
+        QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0);
         QVERIFY(!sigExprWatcher.wasDeleted());
         QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr);
         QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("oddlyNamedNotifySignal()"));
@@ -987,7 +987,7 @@ void tst_qqmlproperty::read()
         QQmlProperty p(&o, "onClicked");
         QCOMPARE(p.read(), QVariant());
 
-        QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
+        QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
         QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p));
 
         QCOMPARE(p.read(), QVariant());
@@ -999,7 +999,7 @@ void tst_qqmlproperty::read()
         QQmlProperty p(&o, "onPropertyWithNotifyChanged");
         QCOMPARE(p.read(), QVariant());
 
-        QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
+        QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
         QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p));
 
         QCOMPARE(p.read(), QVariant());
@@ -1155,7 +1155,7 @@ void tst_qqmlproperty::write()
         QQmlProperty p(&o, "onClicked");
         QCOMPARE(p.write(QVariant("console.log(1921)")), false);
 
-        QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
+        QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
         QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p));
 
         QCOMPARE(p.write(QVariant("console.log(1921)")), false);
@@ -1169,7 +1169,7 @@ void tst_qqmlproperty::write()
         QQmlProperty p(&o, "onPropertyWithNotifyChanged");
         QCOMPARE(p.write(QVariant("console.log(1921)")), false);
 
-        QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
+        QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1)));
         QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p));
 
         QCOMPARE(p.write(QVariant("console.log(1921)")), false);
diff --git a/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml b/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml
new file mode 100644
index 0000000000..705ad07715
--- /dev/null
+++ b/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml
@@ -0,0 +1,22 @@
+import QtQuick 2.0
+
+Rectangle {
+    id: root
+    width: 400; height: 400
+    property int someProp
+
+    states: [
+        State {
+            name: "state1"
+            PropertyChanges { target: root; onSomePropChanged: h1() }
+        },
+        State {
+            name: "state2"
+            PropertyChanges { target: root; onSomePropChanged: h2() }
+        }
+    ]
+
+    // non-default handlers
+    function h1() {}
+    function h2() {}
+}
diff --git a/tests/auto/quick/qquickstates/tst_qquickstates.cpp b/tests/auto/quick/qquickstates/tst_qquickstates.cpp
index 7fd8fc4498..1aca63a628 100644
--- a/tests/auto/quick/qquickstates/tst_qquickstates.cpp
+++ b/tests/auto/quick/qquickstates/tst_qquickstates.cpp
@@ -112,6 +112,7 @@ private slots:
     void signalOverrideCrash();
     void signalOverrideCrash2();
     void signalOverrideCrash3();
+    void signalOverrideCrash4();
     void parentChange();
     void parentChangeErrors();
     void anchorChanges();
@@ -475,6 +476,8 @@ void tst_qquickstates::signalOverride()
         QQuickItemPrivate::get(rect)->setState("green");
         rect->doSomething();
         QCOMPARE(rect->color(),QColor("green"));
+
+        delete rect;
     }
 
     {
@@ -492,6 +495,8 @@ void tst_qquickstates::signalOverride()
         QCOMPARE(rect->color(),QColor("blue"));
         QCOMPARE(innerRect->color(),QColor("green"));
         QCOMPARE(innerRect->property("extendedColor").value<QColor>(),QColor("green"));
+
+        delete rect;
     }
 }
 
@@ -538,6 +543,24 @@ void tst_qquickstates::signalOverrideCrash3()
     delete rect;
 }
 
+void tst_qquickstates::signalOverrideCrash4()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("signalOverrideCrash4.qml"));
+    QQuickRectangle *rect = qobject_cast<QQuickRectangle*>(c.create());
+    QVERIFY(rect != 0);
+
+    QQuickItemPrivate *rectPrivate = QQuickItemPrivate::get(rect);
+
+    rectPrivate->setState("state1");
+    rectPrivate->setState("state2");
+    rectPrivate->setState("state1");
+    rectPrivate->setState("state2");
+    rectPrivate->setState("");
+
+    delete rect;
+}
+
 void tst_qquickstates::parentChange()
 {
     QQmlEngine engine;