QML: Do not leak memory if QQmlData is manipulated from ctor

This creates another QQmlData object on the heap, which is rather
wasteful. However, we cannot really avoid it since it's all too easy to
trigger an operation like this.

If it happens, use the QQmlData it has created, and ignore the memory
we've allocated inline. While we're at it, make the memory ownership a
required argument to the ctor.

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

7 files changed, 61 insertions, 11 deletions

diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 5b7c05c61e..cc13cc7245 100644
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -55,7 +55,9 @@ struct Binding;
 class Q_QML_PRIVATE_EXPORT QQmlData : public QAbstractDeclarativeData
 {
 public:
-    QQmlData();
+    enum Ownership { DoesNotOwnMemory, OwnsMemory };
+
+    QQmlData(Ownership ownership);
     ~QQmlData();
 
     static inline void init() {
@@ -295,7 +297,7 @@ private:
 
     Q_NEVER_INLINE BindingBitsType *growBits(QObject *obj, int bit);
 
-    Q_DISABLE_COPY(QQmlData);
+    Q_DISABLE_COPY_MOVE(QQmlData);
 };
 
 bool QQmlData::wasDeleted(const QObjectPrivate *priv)
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index c9b0912706..a8a185772b 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -255,8 +255,8 @@ void QQmlPrivate::qdeclarativeelement_destructor(QObject *o)
     }
 }
 
-QQmlData::QQmlData()
-    : ownMemory(true), indestructible(true), explicitIndestructibleSet(false),
+QQmlData::QQmlData(Ownership ownership)
+    : ownMemory(ownership == OwnsMemory), indestructible(true), explicitIndestructibleSet(false),
       hasTaintedV4Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
       hasInterceptorMetaObject(false), hasVMEMetaObject(false), hasConstWrapper(false),
       bindingBitsArraySize(InlineBindingArraySize), notifyList(nullptr),
@@ -1525,7 +1525,7 @@ QQmlData *QQmlData::createQQmlData(QObjectPrivate *priv)
 {
     Q_ASSERT(priv);
     Q_ASSERT(!priv->isDeletingChildren);
-    priv->declarativeData = new QQmlData;
+    priv->declarativeData = new QQmlData(OwnsMemory);
     return static_cast<QQmlData *>(priv->declarativeData);
 }
 
diff --git a/src/qml/qml/qqmltype.cpp b/src/qml/qml/qqmltype.cpp
index 56f19f877a..ece6454705 100644
--- a/src/qml/qml/qqmltype.cpp
+++ b/src/qml/qml/qqmltype.cpp
@@ -491,11 +491,10 @@ QObject *QQmlType::createWithQQmlData() const
     auto instance = create(&ddataMemory, sizeof(QQmlData));
     if (!instance)
         return nullptr;
-    QQmlData *ddata = new (ddataMemory) QQmlData;
-    ddata->ownMemory = false;
     QObjectPrivate* p = QObjectPrivate::get(instance);
     Q_ASSERT(!p->isDeletingChildren);
-    p->declarativeData = ddata;
+    if (!p->declarativeData)
+        p->declarativeData = new (ddataMemory) QQmlData(QQmlData::DoesNotOwnMemory);
     return instance;
 }
 
diff --git a/src/qmlmodels/qqmllistmodel.cpp b/src/qmlmodels/qqmllistmodel.cpp
index 14c8a6fd85..3bd180d072 100644
--- a/src/qmlmodels/qqmllistmodel.cpp
+++ b/src/qmlmodels/qqmllistmodel.cpp
@@ -319,9 +319,12 @@ QObject *ListModel::getOrCreateModelObject(QQmlListModel *model, int elementInde
         void *memory = operator new(sizeof(QObject) + sizeof(QQmlData));
         void *ddataMemory = ((char *)memory) + sizeof(QObject);
         e->m_objectCache = new (memory) QObject;
-        QQmlData *ddata = new (ddataMemory) QQmlData;
-        ddata->ownMemory = false;
-        QObjectPrivate::get(e->m_objectCache)->declarativeData = ddata;
+
+        const QAbstractDeclarativeData *old = std::exchange(
+            QObjectPrivate::get(e->m_objectCache)->declarativeData,
+            new (ddataMemory) QQmlData(QQmlData::DoesNotOwnMemory));
+        Q_ASSERT(!old); // QObject should really not manipulate QQmlData
+
         (void)new ModelNodeMetaObject(e->m_objectCache, model, elementIndex);
     }
     return e->m_objectCache;
diff --git a/tests/auto/qml/qqmllanguage/testtypes.cpp b/tests/auto/qml/qqmllanguage/testtypes.cpp
index aff8e50feb..456ffa15bc 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.cpp
+++ b/tests/auto/qml/qqmllanguage/testtypes.cpp
@@ -159,6 +159,7 @@ void registerTypes()
 
     qmlRegisterTypesAndRevisions<Greeter>("QmlOtherThis", 1);
     qmlRegisterTypesAndRevisions<BirthdayParty>("People", 1);
+    qmlRegisterTypesAndRevisions<AttachedInCtor>("Test", 1);
 }
 
 QVariant myCustomVariantTypeConverter(const QString &data)
diff --git a/tests/auto/qml/qqmllanguage/testtypes.h b/tests/auto/qml/qqmllanguage/testtypes.h
index 06d710dfa9..b0ad7ee3af 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.h
+++ b/tests/auto/qml/qqmllanguage/testtypes.h
@@ -18,6 +18,8 @@
 #include <QtQml/qqmlpropertyvaluesource.h>
 #include <QtQml/qqmlscriptstring.h>
 #include <QtQml/qqmlproperty.h>
+
+#include <private/qqmlcomponentattached_p.h>
 #include <private/qqmlcustomparser_p.h>
 
 QVariant myCustomVariantTypeConverter(const QString &data);
@@ -2625,6 +2627,32 @@ public:
     }
 };
 
+class Attachment : public QObject {
+    Q_OBJECT
+public:
+    Attachment(QObject *parent = nullptr) : QObject(parent) {}
+};
+
+class AttachedInCtor : public QObject
+{
+    Q_OBJECT
+    QML_ELEMENT
+    QML_ATTACHED(Attachment)
+
+public:
+    AttachedInCtor(QObject *parent = nullptr)
+        : QObject(parent)
+    {
+        attached = qmlAttachedPropertiesObject<AttachedInCtor>(this, true);
+    }
+
+    static Attachment *qmlAttachedProperties(QObject *object) {
+        return new Attachment(object);
+    }
+
+    QObject *attached = nullptr;
+};
+
 class BirthdayParty : public QObject
 {
     Q_OBJECT
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 5ae0aa7c78..9af21af82c 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -420,6 +420,8 @@ private slots:
     void variantObjectList();
     void jitExceptions();
 
+    void attachedInCtor();
+
 private:
     QQmlEngine engine;
     QStringList defaultImportPathList;
@@ -8076,6 +8078,21 @@ void tst_qqmllanguage::jitExceptions()
     QVERIFY(!o.isNull());
 }
 
+void tst_qqmllanguage::attachedInCtor()
+{
+    QQmlEngine e;
+    QQmlComponent c(&e);
+    c.setData(R"(
+        import Test
+        AttachedInCtor {}
+    )", QUrl());
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    AttachedInCtor *a = qobject_cast<AttachedInCtor *>(o.data());
+    QVERIFY(a->attached);
+    QCOMPARE(a->attached, qmlAttachedPropertiesObject<AttachedInCtor>(a, false));
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"