Fix crash with early QObject property access

In the reported bug, it can happen that we try to access the compile-time resolved
QObject property of an object that is referenced by id. The binding that uses this is
triggered when the property changes but _also_ when the id referenced object gets either
created or deleted. The first time the binding is evaluated is very early on, when the
id referenced object is not created yet, so the binding evaluation fails. However the
dependency is set up, and so later then the id referenced object is created and the id
property is set on the context, the notification triggers and the binding is re-evaluated.
During that binding evaluation a QObject property access happens by index on an object that
doesn't have its VME meta-object set up yet. Therefore the property access fails and a
crash occurs or the Q_ASSERT(property) assertion fails.

The fix is to set register the id named object in the context _after_ the VME meta-object is
setup.

Task-number: QTBUG-40018
Change-Id: Ic2d7b4a0c49635efe68e93f2f6c316eb65f0c309
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

4 files changed, 48 insertions, 5 deletions

diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 1de467b0fa..79c196406f 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -1029,6 +1029,13 @@ void QQmlObjectCreator::recordError(const QV4::CompiledData::Location &location,
     errors << error;
 }
 
+void QQmlObjectCreator::registerObjectWithContextById(int objectIndex, QObject *instance) const
+{
+    QHash<int, int>::ConstIterator idEntry = objectIndexToId.find(objectIndex);
+    if (idEntry != objectIndexToId.constEnd())
+        context->setIdProperty(idEntry.value(), instance);
+}
+
 QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isContextObject)
 {
     QQmlObjectCreationProfiler profiler(sharedState->profiler.profiler);
@@ -1128,10 +1135,6 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
         parserStatus->d = &sharedState->allParserStatusCallbacks.top();
     }
 
-    QHash<int, int>::ConstIterator idEntry = objectIndexToId.find(index);
-    if (idEntry != objectIndexToId.constEnd())
-        context->setIdProperty(idEntry.value(), instance);
-
     // Register the context object in the context early on in order for pending binding
     // initialization to find it available.
     if (isContextObject)
@@ -1146,8 +1149,10 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
         }
     }
 
-    if (isComponent)
+    if (isComponent) {
+        registerObjectWithContextById(index, instance);
         return instance;
+    }
 
     QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.at(index);
     Q_ASSERT(!cache.isNull());
@@ -1312,6 +1317,9 @@ bool QQmlObjectCreator::populateInstance(int index, QObject *instance, QObject *
     } else {
         vmeMetaObject = QQmlVMEMetaObject::get(_qobject);
     }
+
+    registerObjectWithContextById(index, _qobject);
+
     qSwap(_propertyCache, cache);
     qSwap(_vmeMetaObject, vmeMetaObject);
 
diff --git a/src/qml/qml/qqmlobjectcreator_p.h b/src/qml/qml/qqmlobjectcreator_p.h
index 73800ca9df..34a25982db 100644
--- a/src/qml/qml/qqmlobjectcreator_p.h
+++ b/src/qml/qml/qqmlobjectcreator_p.h
@@ -113,6 +113,8 @@ private:
     QString stringAt(int idx) const { return qmlUnit->header.stringAt(idx); }
     void recordError(const QV4::CompiledData::Location &location, const QString &description);
 
+    void registerObjectWithContextById(int objectIndex, QObject *instance) const;
+
     enum Phase {
         Startup,
         CreatingObjects,
diff --git a/tests/auto/qml/qqmllanguage/data/earlyIdObjectAccess.qml b/tests/auto/qml/qqmllanguage/data/earlyIdObjectAccess.qml
new file mode 100644
index 0000000000..22c335b1a7
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/earlyIdObjectAccess.qml
@@ -0,0 +1,23 @@
+import QtQuick 2.0
+
+Item {
+    visible: false
+    property bool success: false
+    property bool loading: true
+
+    Item {
+        visible: someOtherItem.someProperty
+        onVisibleChanged: {
+            if (!visible) {
+                success = loading
+            }
+        }
+    }
+
+    Item {
+        id: someOtherItem
+        property bool someProperty: true
+    }
+
+    Component.onCompleted: loading = false
+}
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 7976987b58..ec931385d5 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -240,6 +240,8 @@ private slots:
 
     void noChildEvents();
 
+    void earlyIdObjectAccess();
+
 private:
     QQmlEngine engine;
     QStringList defaultImportPathList;
@@ -3792,6 +3794,14 @@ void tst_qqmllanguage::noChildEvents()
     QCOMPARE(object->childAddedEventCount(), 0);
 }
 
+void tst_qqmllanguage::earlyIdObjectAccess()
+{
+    QQmlComponent component(&engine, testFileUrl("earlyIdObjectAccess.qml"));
+    QScopedPointer<QObject> o(component.create());
+    QVERIFY(!o.isNull());
+    QVERIFY(o->property("success").toBool());
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"