QtQml: Do not gc objects stored in QML-declared list properties

To this end, store the lists in a somewhat more straight forward way. We
can just use a plain Object since that already has array data. The array
data will integrate with garbage collection as usual.

Pick-to: 6.6 6.5
Fixes: QTBUG-117793
Change-Id: Ia5455fe62c526d69864d5569cd954bad09d4e911
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

4 files changed, 113 insertions, 25 deletions

diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index 27b8af299f..8d524e2e5d 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -56,16 +56,44 @@ QQmlVMEResolvedList::QQmlVMEResolvedList(QQmlListProperty<QObject> *prop)
 
     if (auto *md = static_cast<QV4::MemberData *>(
                 m_metaObject->propertyAndMethodStorage.asManaged())) {
-        const auto *v = (md->data() + m_id)->as<QV4::VariantObject>();
-        Q_ASSERT(v);
-        Q_ASSERT(v->d());
-        QVariant &data = v->d()->data();
-        Q_ASSERT(data.userType() == qMetaTypeId<QVector<QQmlGuard<QObject>>>());
-        m_list = static_cast<QVector<QQmlGuard<QObject>> *>(data.data());
+        const QV4::Value *v = md->data() + m_id;
+        Q_ASSERT(v->as<QV4::Object>());
+        m_list = static_cast<QV4::Heap::Object *>(v->heapObject());
         Q_ASSERT(m_list);
     }
 }
 
+void QQmlVMEResolvedList::append(QObject *o) const
+{
+    QV4::Scope scope(m_list->internalClass->engine);
+    QV4::Heap::ArrayData *arrayData = m_list->arrayData;
+
+    const uint length = arrayData->length();
+    if (Q_UNLIKELY(length == std::numeric_limits<uint>::max())) {
+        scope.engine->throwRangeError(QLatin1String("Too many elements."));
+        return;
+    }
+
+    QV4::ScopedObject object(scope, m_list);
+    QV4::ArrayData::realloc(object, QV4::Heap::ArrayData::Simple, length + 1, false);
+    arrayData->vtable()->put(
+            object, length, QV4::QObjectWrapper::wrap(scope.engine, o));
+}
+
+QObject *QQmlVMEResolvedList::at(qsizetype i) const
+{
+    QV4::Scope scope(m_list->internalClass->engine);
+    QV4::Scoped<QV4::QObjectWrapper> result(scope, m_list->arrayData->get(i));
+    return result ? result->object() : nullptr;
+}
+
+void QQmlVMEResolvedList::replace(qsizetype i, QObject *o) const
+{
+    QV4::Scope scope(m_list->internalClass->engine);
+    QV4::ScopedObject object(scope, m_list);
+    m_list->arrayData->vtable()->put(object, i, QV4::QObjectWrapper::wrap(scope.engine, o));
+}
+
 QQmlVMEResolvedList::~QQmlVMEResolvedList() = default;
 
 void QQmlVMEResolvedList::activateSignal() const
@@ -76,48 +104,48 @@ void QQmlVMEResolvedList::activateSignal() const
 void QQmlVMEMetaObject::list_append(QQmlListProperty<QObject> *prop, QObject *o)
 {
     const QQmlVMEResolvedList resolved(prop);
-    resolved.list()->append(o);
+    resolved.append(o);
     resolved.activateSignal();
 }
 
 void QQmlVMEMetaObject::list_append_nosignal(QQmlListProperty<QObject> *prop, QObject *o)
 {
-    QQmlVMEResolvedList(prop).list()->append(o);
+    QQmlVMEResolvedList(prop).append(o);
 }
 
 static qsizetype list_count(QQmlListProperty<QObject> *prop)
 {
-    return QQmlVMEResolvedList(prop).list()->size();
+    return QQmlVMEResolvedList(prop).size();
 }
 
 static QObject *list_at(QQmlListProperty<QObject> *prop, qsizetype index)
 {
-    return QQmlVMEResolvedList(prop).list()->at(index);
+    return QQmlVMEResolvedList(prop).at(index);
 }
 
 void QQmlVMEMetaObject::list_clear(QQmlListProperty<QObject> *prop)
 {
     const QQmlVMEResolvedList resolved(prop);
-    resolved.list()->clear();
+    resolved.clear();
     resolved.activateSignal();
 }
 
 void QQmlVMEMetaObject::list_clear_nosignal(QQmlListProperty<QObject> *prop)
 {
-    QQmlVMEResolvedList(prop).list()->clear();
+    QQmlVMEResolvedList(prop).clear();
 }
 
 static void list_replace(QQmlListProperty<QObject> *prop, qsizetype index, QObject *o)
 {
     const QQmlVMEResolvedList resolved(prop);
-    resolved.list()->replace(index, o);
+    resolved.replace(index, o);
     resolved.activateSignal();
 }
 
 static void list_removeLast(QQmlListProperty<QObject> *prop)
 {
     const QQmlVMEResolvedList resolved(prop);
-    resolved.list()->removeLast();
+    resolved.removeLast();
     resolved.activateSignal();
 }
 
@@ -648,20 +676,19 @@ QObject* QQmlVMEMetaObject::readPropertyAsQObject(int id) const
     return wrapper->object();
 }
 
-QVector<QQmlGuard<QObject>> *QQmlVMEMetaObject::readPropertyAsList(int id) const
+void QQmlVMEMetaObject::initPropertyAsList(int id) const
 {
     QV4::MemberData *md = propertyAndMethodStorageAsMemberData();
     if (!md)
-        return nullptr;
+        return;
 
     QV4::Scope scope(engine);
-    QV4::Scoped<QV4::VariantObject> v(scope, *(md->data() + id));
-    if (!v || v->d()->data().metaType() != QMetaType::fromType<QVector<QQmlGuard<QObject>>>()) {
-        const QVector<QQmlGuard<QObject>> guards;
-        v = engine->newVariantObject(QMetaType::fromType<QVector<QQmlGuard<QObject>>>(), &guards);
+    QV4::ScopedObject v(scope, *(md->data() + id));
+    if (!v) {
+        v = engine->newObject();
+        v->arrayCreate();
         md->set(engine, id, v);
     }
-    return static_cast<QVector<QQmlGuard<QObject>> *>(v->d()->data().data());
 }
 
 QRectF QQmlVMEMetaObject::readPropertyAsRectF(int id) const
@@ -742,7 +769,7 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void *
                             }
                             quintptr encodedIndex = (inheritanceDepth << idBits) + id;
 
-                            readPropertyAsList(id); // Initializes if necessary
+                            initPropertyAsList(id);
                             *static_cast<QQmlListProperty<QObject> *>(a[0])
                                     = QQmlListProperty<QObject>(
                                         object, reinterpret_cast<void *>(quintptr(encodedIndex)),
diff --git a/src/qml/qml/qqmlvmemetaobject_p.h b/src/qml/qml/qqmlvmemetaobject_p.h
index afbe77c4de..cf91d74e4a 100644
--- a/src/qml/qml/qqmlvmemetaobject_p.h
+++ b/src/qml/qml/qqmlvmemetaobject_p.h
@@ -48,14 +48,38 @@ public:
     ~QQmlVMEResolvedList();
 
     QQmlVMEMetaObject *metaObject() const { return m_metaObject; }
-    QVector<QQmlGuard<QObject>> *list() const { return m_list; }
+    QV4::Heap::Object *list() const { return m_list; }
     quintptr id() const { return m_id; }
 
+    void append(QObject *o) const;
+    void replace(qsizetype i, QObject *o) const;
+    QObject *at(qsizetype i) const;
+
+    qsizetype size() const { return m_list->arrayData->length(); }
+
+    void clear() const
+    {
+        QV4::Scope scope(m_list->internalClass->engine);
+        QV4::ScopedObject object(scope, m_list);
+        m_list->arrayData->vtable()->truncate(object, 0);
+    }
+
+    void removeLast() const
+    {
+        const uint length = m_list->arrayData->length();
+        if (length == 0)
+            return;
+
+        QV4::Scope scope(m_list->internalClass->engine);
+        QV4::ScopedObject object(scope, m_list);
+        m_list->arrayData->vtable()->truncate(object, length - 1);
+    }
+
     void activateSignal() const;
 
 private:
     QQmlVMEMetaObject *m_metaObject = nullptr;
-    QVector<QQmlGuard<QObject>> *m_list = nullptr;
+    QV4::Heap::Object *m_list = nullptr;
     quintptr m_id = 0;
 };
 
@@ -210,7 +234,7 @@ public:
 
     QRectF readPropertyAsRectF(int id) const;
     QObject *readPropertyAsQObject(int id) const;
-    QVector<QQmlGuard<QObject> > *readPropertyAsList(int id) const;
+    void initPropertyAsList(int id) const;
 
     void writeProperty(int id, int v);
     void writeProperty(int id, bool v);
diff --git a/tests/auto/qml/qqmllanguage/data/objectInList.qml b/tests/auto/qml/qqmllanguage/data/objectInList.qml
new file mode 100644
index 0000000000..53c8c3cdd1
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/objectInList.qml
@@ -0,0 +1,17 @@
+import QtQml
+
+QtObject {
+    objectName: "parent"
+    property list<QtObject> child
+    property Component c: QtObject { objectName: "child" }
+
+    function doCreate() {
+        child.push(c.createObject(null));
+    }
+
+    Component.onCompleted: {
+        // Extra function call so that the created object cannot be on the stack
+        doCreate();
+        gc();
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 228b6451f8..d08c73e701 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -434,6 +434,7 @@ private slots:
     void multiVersionSingletons();
     void typeAnnotationCycle();
     void corpseInQmlList();
+    void objectInQmlListAndGc();
 
 private:
     QQmlEngine engine;
@@ -8371,6 +8372,25 @@ void tst_qqmllanguage::corpseInQmlList()
     QCOMPARE(list.count(&list), 0);
 }
 
+void tst_qqmllanguage::objectInQmlListAndGc()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, testFileUrl("objectInList.qml"));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+
+    // Process the deletion event
+    QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+    QCoreApplication::processEvents();
+
+    QQmlListProperty<QObject> children = o->property("child").value<QQmlListProperty<QObject>>();
+    QCOMPARE(children.count(&children), 1);
+    QObject *child = children.at(&children, 0);
+    QVERIFY(child);
+    QCOMPARE(child->objectName(), QLatin1String("child"));
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"