QQmlDMAbstractItemModelData: Guard against deletion during model write

When a `setData` call on a model results in our row being removed
(e.g. when a proxy model filters out the row as a result of this
change), an Instantiator would delete us and we'd try to emit a
property change on garbage `this`.

Amends commit 5db77fb406bb36a27de1ad2e3390720411c833f9which made
Instantiator clean up its objects on a model change.

Pick-to: 6.6 6.5
Change-Id: I5570453de588e32dc5e4235287a83565e5185aa2
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qmlmodels/qqmldmabstractitemmodeldata.cpp

@@ -42,7 +42,11 @@ int QQmlDMAbstractItemModelData::metaCall(QMetaObject::Call call, int id, void *
                 QMetaObject::activate(this, meta, 0, nullptr);
             }
         } else if (*m_type->model) {
+            QQmlGuard<QQmlDMAbstractItemModelData> guard(this);
             setValue(m_type->propertyRoles.at(propertyIndex), *static_cast<QVariant *>(arguments[0]));
+            if (guard.isNull())
+              return -1;
+
             QMetaObject::activate(this, meta, propertyIndex, nullptr);
         }
         emit modelDataChanged();

tests/auto/qml/qqmlinstantiator/data/removeDuringModelChange.qml

@@ -0,0 +1,10 @@
+import QtQuick 2.15
+import QtQml.Models 2.15
+
+Instantiator {
+    delegate: QtObject {
+        function deactivate() {
+            model.active = false;
+        }
+    }
+}

tests/auto/qml/qqmlinstantiator/tst_qqmlinstantiator.cpp

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 #include <qtest.h>
 #include <QSignalSpy>
+#include <QSortFilterProxyModel>
 #include <QDebug>
 
 #include <QtQml/qqmlengine.h>
@@ -28,6 +29,7 @@ private slots:
     void activeModelChangeInteraction();
     void intModelChange();
     void createAndRemove();
+    void removeDuringModelChange();
 
     void asynchronous_data();
     void asynchronous();
@@ -306,6 +308,93 @@ void tst_qqmlinstantiator::boundDelegateComponent()
     QCOMPARE(b->objectAt(2)->objectName(), QStringLiteral("root3"));
 }
 
+class SingleBoolItemModel : public QAbstractListModel
+{
+    Q_OBJECT
+
+public:
+    SingleBoolItemModel(QObject *parent = nullptr) : QAbstractListModel(parent) {}
+
+    int rowCount(const QModelIndex &parent = QModelIndex()) const override
+    {
+        if (parent.isValid())
+            return 0;
+        return 1;
+    }
+
+    QVariant data(const QModelIndex &index, int role) const override
+    {
+        if (index.parent().isValid() || index.row() != 0 || index.column() != 0
+                || role != Qt::UserRole)
+            return QVariant();
+
+        return m_active;
+    }
+
+    bool setData(const QModelIndex &index, const QVariant &value,
+                 int role) override {
+        if (index.parent().isValid() || index.row() != 0 || index.column() != 0
+                || role != Qt::UserRole || m_active == value.toBool())
+            return false;
+
+        m_active = value.toBool();
+        Q_EMIT dataChanged(index, index, QList<int>{Qt::UserRole});
+        return true;
+    }
+
+    QHash<int, QByteArray> roleNames() const override
+    {
+        return { {Qt::UserRole, "active"} };
+    }
+
+private:
+    bool m_active = true;
+};
+
+class FilterBoolRoleProxyModel : public QSortFilterProxyModel
+{
+    Q_OBJECT
+
+public:
+    FilterBoolRoleProxyModel(QObject *parent = nullptr)
+        : QSortFilterProxyModel(parent) {}
+
+    bool filterAcceptsRow(int source_row,
+                          const QModelIndex &source_parent) const override
+    {
+        return sourceModel()->index(source_row, 0, source_parent).data(Qt::UserRole).toBool();
+    }
+};
+
+void tst_qqmlinstantiator::removeDuringModelChange()
+{
+    SingleBoolItemModel model;
+
+    FilterBoolRoleProxyModel proxyModel;
+    proxyModel.setSourceModel(&model);
+    proxyModel.setFilterRole(Qt::UserRole);
+    QCOMPARE(proxyModel.rowCount(), 1);
+
+    QQmlEngine engine;
+    const QUrl url(testFileUrl("removeDuringModelChange.qml"));
+    QQmlComponent component(&engine, url);
+    QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+
+    QScopedPointer<QObject> o(component.create());
+    QVERIFY2(!o.isNull(), qPrintable(component.errorString()));
+
+    QQmlInstantiator *instantiator = qobject_cast<QQmlInstantiator *>(o.data());
+
+    instantiator->setModel(QVariant::fromValue(&proxyModel));
+
+    QSignalSpy removedSpy(instantiator, &QQmlInstantiator::objectRemoved);
+    QMetaObject::invokeMethod(instantiator->object(), "deactivate");
+
+    // We should still be alive at this point.
+    QCOMPARE(removedSpy.size(), 1);
+    QCOMPARE(proxyModel.rowCount(), 0);
+}
+
 QTEST_MAIN(tst_qqmlinstantiator)
 
 #include "tst_qqmlinstantiator.moc"