Fix crash caused by dereferencing collected v8 data

If a var property of a QObject is read after the v8 data associated
with the qobject has been deleted but prior to the DeferredDelete
event being processed, the varProperties array will be null and
a crash will occur.

This patch ensures that we check for this condition in both the
access and set codepaths for var properties, and also ensures
that an object which has previously been queued for deletion cannot
be referenced in JS.

Finally, it adds a unit test to ensure that we don't regress.

Task-number: QTBUG-24748
Change-Id: Idde384ca01e18f4dcf9e376e9379f2c5eb410e14
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

4 files changed, 49 insertions, 1 deletions

diff --git a/tests/auto/qml/qqmlecmascript/data/ComponentWithVarProp.qml b/tests/auto/qml/qqmlecmascript/data/ComponentWithVarProp.qml
new file mode 100644
index 0000000000..3105b8c79f
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/ComponentWithVarProp.qml
@@ -0,0 +1,5 @@
+import QtQuick 2.0
+
+Item {
+    property var varprop: true
+}
diff --git a/tests/auto/qml/qqmlecmascript/data/propertyVarOwnership.5.qml b/tests/auto/qml/qqmlecmascript/data/propertyVarOwnership.5.qml
new file mode 100644
index 0000000000..ad5807b1cf
--- /dev/null
+++ b/tests/auto/qml/qqmlecmascript/data/propertyVarOwnership.5.qml
@@ -0,0 +1,28 @@
+import QtQuick 2.0
+import Qt.test 1.0 as ModuleApi
+
+Item {
+    id: testOwnership
+    property bool test: false
+
+    function runTest() {
+        var o;
+        var c = Qt.createComponent("ComponentWithVarProp.qml");
+        if (c.status == Component.Ready) {
+            o = c.createObject();
+        } else {
+            return; // failed to create component.
+        }
+        o.varprop = true;                // causes initialization of varProperties.
+        ModuleApi.trackObject(o);        // stores QObject ptr
+        if (ModuleApi.trackedObject() == null) return;        // is still valid, should have a valid v8object.
+        o = new Date();                  // causes object to be gc-able.
+        gc();  // collect object's v8object + varProperties, queues deleteLater.
+        if (ModuleApi.trackedObject() != null) return;        // v8object was previously collected.
+        ModuleApi.setTrackedObjectProperty("varprop");        // deferences varProperties of object.
+        test = !(ModuleApi.trackedObjectProperty("varprop")); // deferences varProperties of object.
+        // if we didn't crash, success.
+    }
+}
+
+
diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index 65b84d66cb..54fab26405 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.h
+++ b/tests/auto/qml/qqmlecmascript/testtypes.h
@@ -982,7 +982,7 @@ class testQObjectApi : public QObject
 
 public:
     testQObjectApi(QObject* parent = 0)
-        : QObject(parent), m_testProperty(0), m_testWritableProperty(0), m_testWritableFinalProperty(0), m_methodCallCount(0)
+        : QObject(parent), m_testProperty(0), m_testWritableProperty(0), m_testWritableFinalProperty(0), m_methodCallCount(0), m_trackedObject(0)
     {
     }
 
@@ -992,6 +992,11 @@ public:
     Q_INVOKABLE int qobjectEnumTestMethod(MyEnum val) { return (static_cast<int>(val) + 5); }
     Q_INVOKABLE int qobjectTestMethod(int increment = 1) { m_methodCallCount += increment; return m_methodCallCount; }
 
+    Q_INVOKABLE void trackObject(QObject *obj) { m_trackedObject = obj; }
+    Q_INVOKABLE QObject *trackedObject() const { return m_trackedObject; }
+    Q_INVOKABLE void setTrackedObjectProperty(const QString &propName) const { m_trackedObject->setProperty(qPrintable(propName), QVariant(5)); }
+    Q_INVOKABLE QVariant trackedObjectProperty(const QString &propName) const { return m_trackedObject->property(qPrintable(propName)); }
+
     int qobjectTestProperty() const { return m_testProperty; }
     void setQObjectTestProperty(int tp) { m_testProperty = tp; emit qobjectTestPropertyChanged(tp); }
 
@@ -1011,6 +1016,7 @@ private:
     int m_testWritableProperty;
     int m_testWritableFinalProperty;
     int m_methodCallCount;
+    QObject *m_trackedObject;
 };
 
 class CircularReferenceObject : public QObject,
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index cca008eb0c..3bbbc2c79c 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -3906,6 +3906,15 @@ void tst_qqmlecmascript::propertyVarOwnership()
 
     delete object;
     }
+    // Garbage collection cannot result in attempted dereference of empty handle
+    {
+    QQmlComponent component(&engine, testFileUrl("propertyVarOwnership.5.qml"));
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+    QMetaObject::invokeMethod(object, "runTest");
+    QCOMPARE(object->property("test").toBool(), true);
+    delete object;
+    }
 }
 
 void tst_qqmlecmascript::propertyVarImplicitOwnership()