From 60d66bea87b72e66e7c466e1c27f966762e1fd5a Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Tue, 11 Feb 2014 16:33:33 -0600 Subject: 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 Reviewed-by: Lars Knoll --- src/qml/qml/qqmljavascriptexpression.cpp | 113 ++++++++++++++++--------------- 1 file changed, 58 insertions(+), 55 deletions(-) (limited to 'src/qml/qml/qqmljavascriptexpression.cpp') diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp index 499ade1ca5..9453d455e7 100644 --- 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()) { -- cgit v1.2.3