Fix QDeclarativePropertyCache crash.

Reserve enough space in the signalHandlerIndexCache so that it will
not reallocated while the property cache is being built as this will
invalidate the pointers stored in the stringCache.  Also ensure
signals for all cached meta-objects are included in
signalHandlerIndexCache, and don't over allocate propertyIndexCache
and methodIndexCache.

Change-Id: Ic285d832d4b86106176bfe723ff10bdd65143910
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

diff --git a/src/declarative/qml/qdeclarativepropertycache.cpp b/src/declarative/qml/qdeclarativepropertycache.cpp
index 239b794..67d0cea 100644 (file)
--- a/src/declarative/qml/qdeclarativepropertycache.cpp
+++ b/src/declarative/qml/qdeclarativepropertycache.cpp
@@ -117,6 +117,14 @@ static QDeclarativePropertyData::Flags flagsForPropertyType(int propType, QDecla
     return flags;
 }
 
+static int metaObjectSignalCount(const QMetaObject *metaObject)
+{
+    int signalCount = 0;
+    for (const QMetaObject *obj = metaObject; obj; obj = obj->superClass())
+        signalCount += QMetaObjectPrivate::get(obj)->signalCount;
+    return signalCount;
+}
+
 QDeclarativePropertyData::Flags
 QDeclarativePropertyData::flagsForProperty(const QMetaProperty &p, QDeclarativeEngine *engine)
 {
@@ -220,8 +228,8 @@ void QDeclarativePropertyData::lazyLoad(const QMetaMethod &m)
 Creates a new empty QDeclarativePropertyCache.
 */
 QDeclarativePropertyCache::QDeclarativePropertyCache(QDeclarativeEngine *e)
-: engine(e), parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0), metaObject(0), 
-  argumentsCache(0)
+: engine(e), parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0),
+  signalHanderIndexCacheStart(0), metaObject(0), argumentsCache(0)
 {
     Q_ASSERT(engine);
 }
@@ -230,8 +238,8 @@ QDeclarativePropertyCache::QDeclarativePropertyCache(QDeclarativeEngine *e)
 Creates a new QDeclarativePropertyCache of \a metaObject.
 */
 QDeclarativePropertyCache::QDeclarativePropertyCache(QDeclarativeEngine *e, const QMetaObject *metaObject)
-: engine(e), parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0), metaObject(0),
-  argumentsCache(0)
+: engine(e), parent(0), propertyIndexCacheStart(0), methodIndexCacheStart(0),
+  signalHanderIndexCacheStart(0), metaObject(0), argumentsCache(0)
 {
     Q_ASSERT(engine);
     Q_ASSERT(metaObject);
@@ -279,6 +287,7 @@ QDeclarativePropertyCache *QDeclarativePropertyCache::copy(int reserve)
     cache->parent->addref();
     cache->propertyIndexCacheStart = propertyIndexCache.count() + propertyIndexCacheStart;
     cache->methodIndexCacheStart = methodIndexCache.count() + methodIndexCacheStart;
+    cache->signalHanderIndexCacheStart = signalHandlerIndexCache.count() + signalHanderIndexCacheStart;
     cache->stringCache.copyAndReserve(stringCache, reserve);
     cache->allowedRevisionCache = allowedRevisionCache;
     cache->metaObject = metaObject;
@@ -313,7 +322,7 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb
 
     int methodCount = metaObject->methodCount();
     Q_ASSERT(QMetaObjectPrivate::get(metaObject)->revision >= 4);
-    int signalCount = QMetaObjectPrivate::get(metaObject)->signalCount;
+    int signalCount = metaObjectSignalCount(metaObject);
     int classInfoCount = QMetaObjectPrivate::get(metaObject)->classInfoCount;
 
     QDeclarativeAccessorProperties::Properties accessorProperties;
@@ -350,10 +359,13 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb
 
     // 3 to block the destroyed signal and the deleteLater() slot
     int methodOffset = qMax(3, metaObject->methodOffset()); 
+    int signalOffset = signalCount - QMetaObjectPrivate::get(metaObject)->signalCount;
 
+    // update() should have reserved enough space in the vector that this doesn't cause a realloc
+    // and invalidate the stringCache.
     methodIndexCache.resize(methodCount - methodIndexCacheStart);
-    signalHandlerIndexCache.resize(signalCount);
-    int signalHandlerIndex = 0;
+    signalHandlerIndexCache.resize(signalCount - signalHanderIndexCacheStart);
+    int signalHandlerIndex = signalOffset;
     for (int ii = methodOffset; ii < methodCount; ++ii) {
         QMetaMethod m = metaObject->method(ii);
         if (m.access() == QMetaMethod::Private) 
@@ -386,7 +398,7 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb
         data->metaObjectOffset = allowedRevisionCache.count() - 1;
 
         if (data->isSignal()) {
-            sigdata = &signalHandlerIndexCache[signalHandlerIndex];
+            sigdata = &signalHandlerIndexCache[signalHandlerIndex - signalHanderIndexCacheStart];
             *sigdata = *data;
             sigdata->flags |= QDeclarativePropertyData::IsSignalHandler;
         }
@@ -439,6 +451,8 @@ void QDeclarativePropertyCache::append(QDeclarativeEngine *engine, const QMetaOb
     int propCount = metaObject->propertyCount();
     int propOffset = metaObject->propertyOffset();
 
+    // update() should have reserved enough space in the vector that this doesn't cause a realloc
+    // and invalidate the stringCache.
     propertyIndexCache.resize(propCount - propertyIndexCacheStart);
     for (int ii = propOffset; ii < propCount; ++ii) {
         QMetaProperty p = metaObject->property(ii);
@@ -524,13 +538,19 @@ void QDeclarativePropertyCache::update(QDeclarativeEngine *engine, const QMetaOb
     Q_ASSERT(metaObject);
     Q_ASSERT(stringCache.isEmpty());
 
-    // Optimization to prevent unnecessary reallocation of lists
+    // Preallocate enough space in the index caches for all the properties/methods/signals that
+    // are not cached in a parent cache so that the caches never need to be reallocated as this
+    // would invalidate pointers stored in the stringCache.
     int pc = metaObject->propertyCount();
     int mc = metaObject->methodCount();
-    propertyIndexCache.reserve(pc);
-    methodIndexCache.reserve(mc);
-
-    stringCache.reserve(pc + mc);
+    int sc = metaObjectSignalCount(metaObject);
+    propertyIndexCache.reserve(pc - propertyIndexCacheStart);
+    methodIndexCache.reserve(mc - methodIndexCacheStart);
+    signalHandlerIndexCache.reserve(sc - signalHanderIndexCacheStart);
+
+    // Reserve enough space in the stringCache for all properties/methods/signals including those
+    // cached in a parent cache.
+    stringCache.reserve(pc + mc + sc);
 
     updateRecur(engine,metaObject);
 }

diff --git a/src/declarative/qml/qdeclarativepropertycache_p.h b/src/declarative/qml/qdeclarativepropertycache_p.h
index 3deafdf..746c1fe 100644 (file)
--- a/src/declarative/qml/qdeclarativepropertycache_p.h
+++ b/src/declarative/qml/qdeclarativepropertycache_p.h
@@ -272,6 +272,7 @@ private:
     QDeclarativePropertyCache *parent;
     int propertyIndexCacheStart;
     int methodIndexCacheStart;
+    int signalHanderIndexCacheStart;
 
     IndexCache propertyIndexCache;
     IndexCache methodIndexCache;

diff --git a/tests/auto/declarative/qdeclarativepropertycache/qdeclarativepropertycache.pro b/tests/auto/declarative/qdeclarativepropertycache/qdeclarativepropertycache.pro
new file mode 100644 (file)
index 0000000..4b47a2c
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativepropertycache/qdeclarativepropertycache.pro
@@ -0,0 +1,8 @@
+CONFIG += testcase
+TARGET = tst_qdeclarativepropertycache
+macx:CONFIG -= app_bundle
+
+SOURCES += tst_qdeclarativepropertycache.cpp
+
+CONFIG += parallel_test
+QT += core-private gui-private declarative-private testlib v8-private

diff --git a/tests/auto/declarative/qdeclarativepropertycache/tst_qdeclarativepropertycache.cpp b/tests/auto/declarative/qdeclarativepropertycache/tst_qdeclarativepropertycache.cpp
new file mode 100644 (file)
index 0000000..ba1140a
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativepropertycache/tst_qdeclarativepropertycache.cpp
@@ -0,0 +1,281 @@
+/****************************************************************************
+**
+** Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies).
+** All rights reserved.
+** Contact: Nokia Corporation (qt-info@nokia.com)
+**
+** This file is part of the test suite 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$
+**
+****************************************************************************/
+
+#include <qtest.h>
+#include <private/qdeclarativepropertycache_p.h>
+#include <QtDeclarative/qdeclarativeengine.h>
+#include "../../shared/util.h"
+
+class tst_qdeclarativepropertycache : public QObject
+{
+    Q_OBJECT
+public:
+    tst_qdeclarativepropertycache() {}
+
+private slots:
+    void properties();
+    void propertiesDerived();
+    void methods();
+    void methodsDerived();
+    void signalHandlers();
+    void signalHandlersDerived();
+
+private:
+    QDeclarativeEngine engine;
+};
+
+class BaseObject : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int propertyA READ propertyA NOTIFY propertyAChanged)
+    Q_PROPERTY(QString propertyB READ propertyB NOTIFY propertyBChanged)
+public:
+    BaseObject(QObject *parent = 0) : QObject(parent) {}
+
+    int propertyA() const { return 0; }
+    QString propertyB() const { return QString(); }
+
+public Q_SLOTS:
+    void slotA() {}
+
+Q_SIGNALS:
+    void propertyAChanged();
+    void propertyBChanged();
+    void signalA();
+};
+
+class DerivedObject : public BaseObject
+{
+    Q_OBJECT
+    Q_PROPERTY(int propertyC READ propertyC NOTIFY propertyCChanged)
+    Q_PROPERTY(QString propertyD READ propertyD NOTIFY propertyDChanged)
+public:
+    DerivedObject(QObject *parent = 0) : BaseObject(parent) {}
+
+    int propertyC() const { return 0; }
+    QString propertyD() const { return QString(); }
+
+public Q_SLOTS:
+    void slotB() {}
+
+Q_SIGNALS:
+    void propertyCChanged();
+    void propertyDChanged();
+    void signalB();
+};
+
+void tst_qdeclarativepropertycache::properties()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(new QDeclarativePropertyCache(&engine, metaObject));
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("propertyA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyA"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyB"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyC")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyC"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyD")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyD"));
+}
+
+void tst_qdeclarativepropertycache::propertiesDerived()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> parentCache(new QDeclarativePropertyCache(&engine, &BaseObject::staticMetaObject));
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(parentCache->copy());
+    cache->append(&engine, object.metaObject());
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("propertyA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyA"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyB"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyC")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyC"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyD")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfProperty("propertyD"));
+}
+
+void tst_qdeclarativepropertycache::methods()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(new QDeclarativePropertyCache(&engine, metaObject));
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("slotA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("slotA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("slotB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("slotB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("signalA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("signalB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyAChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyAChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyBChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyBChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyCChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyCChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyDChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyDChanged()"));
+}
+
+void tst_qdeclarativepropertycache::methodsDerived()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> parentCache(new QDeclarativePropertyCache(&engine, &BaseObject::staticMetaObject));
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(parentCache->copy());
+    cache->append(&engine, object.metaObject());
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("slotA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("slotA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("slotB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("slotB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("signalA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("signalB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyAChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyAChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyBChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyBChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyCChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyCChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("propertyDChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyDChanged()"));
+}
+
+void tst_qdeclarativepropertycache::signalHandlers()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(new QDeclarativePropertyCache(&engine, metaObject));
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("onSignalA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onSignalB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyAChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyAChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyBChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyBChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyCChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyCChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyDChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyDChanged()"));
+}
+
+void tst_qdeclarativepropertycache::signalHandlersDerived()
+{
+    QDeclarativeEngine engine;
+    DerivedObject object;
+    const QMetaObject *metaObject = object.metaObject();
+
+    QDeclarativeRefPointer<QDeclarativePropertyCache> parentCache(new QDeclarativePropertyCache(&engine, &BaseObject::staticMetaObject));
+    QDeclarativeRefPointer<QDeclarativePropertyCache> cache(parentCache->copy());
+    cache->append(&engine, object.metaObject());
+    QDeclarativePropertyData *data;
+
+    QVERIFY(data = cache->property(QLatin1String("onSignalA")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalA()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onSignalB")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("signalB()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyAChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyAChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyBChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyBChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyCChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyCChanged()"));
+
+    QVERIFY(data = cache->property(QLatin1String("onPropertyDChanged")));
+    QCOMPARE(data->coreIndex, metaObject->indexOfMethod("propertyDChanged()"));
+}
+
+QTEST_MAIN(tst_qdeclarativepropertycache)
+
+#include "tst_qdeclarativepropertycache.moc"