Fix crash when the currently running binding is deleted.

Stop capturing properties once the expression is deleted. The
following example triggers invalid read/write memcheck errors
when trying to capture propFromParentScope:

Item {
    property real testProp: {
        if (x == 0) testProp = 7
        return propFromParentScope
    }
}

Which can eventually lead to a crash.

Task-number: QTBUG-36798
Change-Id: I233de2c81498884df0563e8ce155752845aafcfb
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp
index 499ade1..9453d45 100644 (file)
--- a/src/qml/qml/qqmljavascriptexpression.cpp
+++ b/src/qml/qml/qqmljavascriptexpression.cpp
@@ -138,8 +138,12 @@ QV4::ReturnedValue QQmlJavaScriptExpression::evaluate(QQmlContextData *context,
 
     QQmlEnginePrivate *ep = QQmlEnginePrivate::get(context->engine);
 
+    // All code that follows must check with watcher before it accesses data members
+    // incase we have been deleted.
+    DeleteWatcher watcher(this);
+
     Q_ASSERT(notifyOnValueChanged() || activeGuards.isEmpty());
-    GuardCapture capture(context->engine, this);
+    GuardCapture capture(context->engine, this, &watcher);
 
     QQmlEnginePrivate::PropertyCapture *lastPropertyCapture = ep->propertyCapture;
     ep->propertyCapture = notifyOnValueChanged()?&capture:0;
@@ -148,10 +152,6 @@ QV4::ReturnedValue QQmlJavaScriptExpression::evaluate(QQmlContextData *context,
     if (notifyOnValueChanged())
         capture.guards.copyAndClearPrepend(activeGuards);
 
-    // All code that follows must check with watcher before it accesses data members
-    // incase we have been deleted.
-    DeleteWatcher watcher(this);
-
     QV4::ExecutionEngine *v4 = QV8Engine::getV4(ep->v8engine());
     QV4::Scope scope(v4);
     QV4::ScopedValue result(scope, QV4::Primitive::undefinedValue());
@@ -196,72 +196,75 @@ QV4::ReturnedValue QQmlJavaScriptExpression::evaluate(QQmlContextData *context,
 
 void QQmlJavaScriptExpression::GuardCapture::captureProperty(QQmlNotifier *n)
 {
-    if (expression) {
+    if (watcher->wasDeleted())
+        return;
+
+    Q_ASSERT(expression);
+    // Try and find a matching guard
+    while (!guards.isEmpty() && !guards.first()->isConnected(n))
+        guards.takeFirst()->Delete();
+
+    Guard *g = 0;
+    if (!guards.isEmpty()) {
+        g = guards.takeFirst();
+        g->cancelNotify();
+        Q_ASSERT(g->isConnected(n));
+    } else {
+        g = Guard::New(expression, engine);
+        g->connect(n);
+    }
+
+    expression->activeGuards.prepend(g);
+}
+
+/*! \internal
+    \reimp
+
+    \a n is in the signal index range (see QObjectPrivate::signalIndex()).
+*/
+void QQmlJavaScriptExpression::GuardCapture::captureProperty(QObject *o, int c, int n)
+{
+    if (watcher->wasDeleted())
+        return;
+
+    Q_ASSERT(expression);
+    if (n == -1) {
+        if (!errorString) {
+            errorString = new QStringList;
+            QString preamble = QLatin1String("QQmlExpression: Expression ") +
+                    expression->m_vtable->expressionIdentifier(expression) +
+                    QLatin1String(" depends on non-NOTIFYable properties:");
+            errorString->append(preamble);
+        }
+
+        const QMetaObject *metaObj = o->metaObject();
+        QMetaProperty metaProp = metaObj->property(c);
+
+        QString error = QLatin1String("    ") +
+                QString::fromUtf8(metaObj->className()) +
+                QLatin1String("::") +
+                QString::fromUtf8(metaProp.name());
+        errorString->append(error);
+    } else {
 
         // Try and find a matching guard
-        while (!guards.isEmpty() && !guards.first()->isConnected(n))
+        while (!guards.isEmpty() && !guards.first()->isConnected(o, n))
             guards.takeFirst()->Delete();
 
         Guard *g = 0;
         if (!guards.isEmpty()) {
             g = guards.takeFirst();
             g->cancelNotify();
-            Q_ASSERT(g->isConnected(n));
+            Q_ASSERT(g->isConnected(o, n));
         } else {
             g = Guard::New(expression, engine);
-            g->connect(n);
+            g->connect(o, n, engine);
         }
 
         expression->activeGuards.prepend(g);
     }
 }
 
-/*! \internal
-    \reimp
-
-    \a n is in the signal index range (see QObjectPrivate::signalIndex()).
-*/
-void QQmlJavaScriptExpression::GuardCapture::captureProperty(QObject *o, int c, int n)
-{
-    if (expression) {
-        if (n == -1) {
-            if (!errorString) {
-                errorString = new QStringList;
-                QString preamble = QLatin1String("QQmlExpression: Expression ") +
-                                   expression->m_vtable->expressionIdentifier(expression) +
-                                   QLatin1String(" depends on non-NOTIFYable properties:");
-                errorString->append(preamble);
-            }
-
-            const QMetaObject *metaObj = o->metaObject();
-            QMetaProperty metaProp = metaObj->property(c);
-
-            QString error = QLatin1String("    ") +
-                            QString::fromUtf8(metaObj->className()) +
-                            QLatin1String("::") +
-                            QString::fromUtf8(metaProp.name());
-            errorString->append(error);
-        } else {
-
-            // Try and find a matching guard
-            while (!guards.isEmpty() && !guards.first()->isConnected(o, n))
-                guards.takeFirst()->Delete();
-
-            Guard *g = 0;
-            if (!guards.isEmpty()) {
-                g = guards.takeFirst();
-                g->cancelNotify();
-                Q_ASSERT(g->isConnected(o, n));
-            } else {
-                g = Guard::New(expression, engine);
-                g->connect(o, n, engine);
-            }
-
-            expression->activeGuards.prepend(g);
-        }
-    }
-}
-
 void QQmlJavaScriptExpression::clearError()
 {
     if (m_vtable.hasValue()) {

diff --git a/src/qml/qml/qqmljavascriptexpression_p.h b/src/qml/qml/qqmljavascriptexpression_p.h
index 7d65f1c..d0f10e4 100644 (file)
--- a/src/qml/qml/qqmljavascriptexpression_p.h
+++ b/src/qml/qml/qqmljavascriptexpression_p.h
@@ -161,8 +161,8 @@ private:
     friend void QQmlJavaScriptExpressionGuard_callback(QQmlNotifierEndpoint *, void **);
 
     struct GuardCapture : public QQmlEnginePrivate::PropertyCapture {
-        GuardCapture(QQmlEngine *engine, QQmlJavaScriptExpression *e)
-        : engine(engine), expression(e), errorString(0) { }
+        GuardCapture(QQmlEngine *engine, QQmlJavaScriptExpression *e, DeleteWatcher *w)
+        : engine(engine), expression(e), watcher(w), errorString(0) { }
 
         ~GuardCapture()  {
             Q_ASSERT(guards.isEmpty());
@@ -174,6 +174,7 @@ private:
 
         QQmlEngine *engine;
         QQmlJavaScriptExpression *expression;
+        DeleteWatcher *watcher;
         QFieldList<Guard, &Guard::next> guards;
         QStringList *errorString;
     };