Hold internal reference to incubator object while incubating

Task-number: QTBUG-53111
Change-Id: Ifaef6a855914d79155f8028b0de7ccca3c9a00f5
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

3 files changed, 60 insertions, 7 deletions

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index a67c5c4a2b..5ed3cc5d6a 100644
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -1132,22 +1132,23 @@ class QQmlComponentIncubator : public QQmlIncubator
 public:
     QQmlComponentIncubator(QV4::Heap::QmlIncubatorObject *inc, IncubationMode mode)
         : QQmlIncubator(mode)
-        , incubatorObject(inc)
-    {}
+    {
+        incubatorObject.set(inc->internalClass->engine, inc);
+    }
 
     void statusChanged(Status s) override {
-        QV4::Scope scope(incubatorObject->internalClass->engine);
-        QV4::Scoped<QV4::QmlIncubatorObject> i(scope, incubatorObject);
+        QV4::Scope scope(incubatorObject.engine());
+        QV4::Scoped<QV4::QmlIncubatorObject> i(scope, incubatorObject.as<QV4::QmlIncubatorObject>());
         i->statusChanged(s);
     }
 
     void setInitialState(QObject *o) override {
-        QV4::Scope scope(incubatorObject->internalClass->engine);
-        QV4::Scoped<QV4::QmlIncubatorObject> i(scope, incubatorObject);
+        QV4::Scope scope(incubatorObject.engine());
+        QV4::Scoped<QV4::QmlIncubatorObject> i(scope, incubatorObject.as<QV4::QmlIncubatorObject>());
         i->setInitialState(o);
     }
 
-    QV4::Heap::QmlIncubatorObject *incubatorObject;
+    QV4::PersistentValue incubatorObject; // keep a strong internal reference while incubating
 };
 
 
@@ -1571,6 +1572,9 @@ void QV4::QmlIncubatorObject::statusChanged(QQmlIncubator::Status s)
             QQmlEnginePrivate::warning(QQmlEnginePrivate::get(scope.engine->qmlEngine()), error);
         }
     }
+
+    if (s != QQmlIncubator::Loading)
+        d()->incubator->incubatorObject.clear();
 }
 
 #undef INITIALPROPERTIES_SOURCE
diff --git a/tests/auto/qml/qqmlincubator/data/garbageCollection.qml b/tests/auto/qml/qqmlincubator/data/garbageCollection.qml
new file mode 100644
index 0000000000..6866a02a00
--- /dev/null
+++ b/tests/auto/qml/qqmlincubator/data/garbageCollection.qml
@@ -0,0 +1,19 @@
+import QtQuick 2.0
+
+QtObject {
+    id: root
+
+    property var incubator
+
+    function getAndClearIncubator() {
+        var result = incubator
+        incubator = null
+        return result
+    }
+
+    Component.onCompleted: {
+        var c = Qt.createComponent("statusChanged.qml"); // use existing simple type for convenience
+        var incubator = c.incubateObject(root);
+        incubator.onStatusChanged = function(status) { if (status === 1) root.incubator = incubator }
+    }
+}
diff --git a/tests/auto/qml/qqmlincubator/tst_qqmlincubator.cpp b/tests/auto/qml/qqmlincubator/tst_qqmlincubator.cpp
index 8f0e04e12e..8e25079703 100644
--- a/tests/auto/qml/qqmlincubator/tst_qqmlincubator.cpp
+++ b/tests/auto/qml/qqmlincubator/tst_qqmlincubator.cpp
@@ -39,6 +39,7 @@
 #include <QQmlComponent>
 #include <QQmlIncubator>
 #include "../../shared/util.h"
+#include <private/qjsvalue_p.h>
 #include <private/qqmlincubator_p.h>
 #include <private/qqmlobjectcreator_p.h>
 
@@ -68,6 +69,7 @@ private slots:
     void chainedAsynchronousClear();
     void selfDelete();
     void contextDelete();
+    void garbageCollection();
 
 private:
     QQmlIncubationController controller;
@@ -1144,6 +1146,34 @@ void tst_qqmlincubator::contextDelete()
     }
 }
 
+// QTBUG-53111
+void tst_qqmlincubator::garbageCollection()
+{
+    QQmlComponent component(&engine, testFileUrl("garbageCollection.qml"));
+    QScopedPointer<QObject> obj(component.create());
+
+    engine.collectGarbage();
+
+    bool b = true;
+    controller.incubateWhile(&b);
+
+    // verify incubation completed (the incubator was not prematurely collected)
+    QVariant incubatorVariant;
+    QMetaObject::invokeMethod(obj.data(), "getAndClearIncubator", Q_RETURN_ARG(QVariant, incubatorVariant));
+    QJSValue strongRef = incubatorVariant.value<QJSValue>();
+    QVERIFY(!strongRef.isNull() && !strongRef.isUndefined());
+
+    // turn the last strong reference to the incubator into a weak one and collect
+    QV4::WeakValue weakIncubatorRef;
+    weakIncubatorRef.set(QQmlEnginePrivate::getV4Engine(&engine), *QJSValuePrivate::getValue(&strongRef));
+    strongRef = QJSValue();
+    incubatorVariant.clear();
+
+    // verify incubator is correctly collected now that incubation is complete and all references are gone
+    engine.collectGarbage();
+    QVERIFY(weakIncubatorRef.isNullOrUndefined());
+}
+
 QTEST_MAIN(tst_qqmlincubator)
 
 #include "tst_qqmlincubator.moc"