aboutsummaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorMitch Curtis <[email protected]>2025-03-07 14:34:32 +0800
committerMitch Curtis <[email protected]>2025-07-04 13:02:04 +0800
commitfa8bfd6ed5ec4150cd2b05cd17b199097193fb1b (patch)
treec7245941420d0ea7b3471658cc54e185ff982930 /tests
parent2e1337d87a60b8d1a30f5296bf3f614ec0bca383 (diff)
Fix heap-use-after-free in Container when using ListView transitionsHEADdev
Container's removeItem function removes and deletes items immediately. This has caused issues in the past, like QTBUG-46798. QTBUG-133256 is similar to that, except it also involves transitions. This patch fixes the crash by listening for the destruction of items created within an ObjectModel and removing them so that views don't have dangling pointers to them. Add removeAndDestroyObjectModelItem to tst_qquicklistview2, which is similar to the test added for 1e3924d8f585dd9099eb74ffbc17950c693d14da but adds more thorough checks and transitions. Fixes: QTBUG-133256 Task-number: QTBUG-46798 Pick-to: 6.5 6.8 6.9 6.10 Change-Id: I9777713edfc6f82a4e9edefe2fd6c9831f03c261 Reviewed-by: Shawn Rutledge <[email protected]> Reviewed-by: Richard Moe Gustavsen <[email protected]>
Diffstat (limited to 'tests')
-rw-r--r--tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml30
-rw-r--r--tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml38
-rw-r--r--tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp53
-rw-r--r--tests/auto/quickcontrols/controls/data/tst_container.qml74
4 files changed, 195 insertions, 0 deletions
diff --git a/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml
new file mode 100644
index 0000000000..994bd8400b
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItem.qml
@@ -0,0 +1,30 @@
+// Copyright (C) 2025 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
+
+import QtQml.Models
+import QtQuick
+
+ListView {
+ width: 200
+ height: 200
+
+ model: ObjectModel {
+ id: objectModel
+
+ Rectangle {
+ width: 200
+ height: 200
+ color: "red"
+ }
+ Rectangle {
+ width: 200
+ height: 200
+ color: "green"
+ }
+ Rectangle {
+ width: 200
+ height: 200
+ color: "blue"
+ }
+ }
+}
diff --git a/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml
new file mode 100644
index 0000000000..b979bfd118
--- /dev/null
+++ b/tests/auto/quick/qquicklistview2/data/removeAndDestroyObjectModelItemWithTransitions.qml
@@ -0,0 +1,38 @@
+// Copyright (C) 2025 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
+
+import QtQml.Models
+import QtQuick
+
+ListView {
+ width: 200
+ height: 200
+
+ displaced: Transition {
+ NumberAnimation { properties: "x,y"; duration: 400; easing.type: Easing.OutBounce }
+ }
+
+ remove: Transition {
+ NumberAnimation { properties: "scale"; from: 1; to: 0; duration: 400 }
+ }
+
+ model: ObjectModel {
+ id: objectModel
+
+ Rectangle {
+ width: 200
+ height: 200
+ color: "red"
+ }
+ Rectangle {
+ width: 200
+ height: 200
+ color: "green"
+ }
+ Rectangle {
+ width: 200
+ height: 200
+ color: "blue"
+ }
+ }
+}
diff --git a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
index ed95f06f26..94feee92f3 100644
--- a/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
+++ b/tests/auto/quick/qquicklistview2/tst_qquicklistview2.cpp
@@ -4,6 +4,7 @@
#include "delegatemodelkinds.h"
#include <QtTest/QTest>
+#include <QtQmlModels/private/qqmlobjectmodel_p.h>
#include <QtQuick/qquickview.h>
#include <QtQuick/private/qquickitemview_p_p.h>
#include <QtQuick/private/qquicklistview_p.h>
@@ -80,6 +81,8 @@ private slots:
void delegateModelAccess_data();
void delegateModelAccess();
+ void removeAndDestroyObjectModelItem_data();
+ void removeAndDestroyObjectModelItem();
private:
void flickWithTouch(QQuickWindow *window, const QPoint &from, const QPoint &to);
@@ -1480,6 +1483,56 @@ void tst_QQuickListView2::delegateModelAccess()
QCOMPARE(delegate->property("modelX").toDouble(), expected);
}
+enum RemovalPolicy {
+ RemoveAndDestroy,
+ OnlyDestroy
+};
+
+void tst_QQuickListView2::removeAndDestroyObjectModelItem_data()
+{
+ QTest::addColumn<QString>("qmlFilePath");
+ QTest::addColumn<RemovalPolicy>("removalPolicy");
+
+ QTest::addRow("remove and destroy, no transitions")
+ << "removeAndDestroyObjectModelItem.qml" << RemoveAndDestroy;
+ QTest::addRow("destroy, no transitions")
+ << "removeAndDestroyObjectModelItem.qml" << OnlyDestroy;
+ QTest::addRow("remove and destroy, transitions")
+ << "removeAndDestroyObjectModelItemWithTransitions.qml" << RemoveAndDestroy;
+ QTest::addRow("destroy, transitions")
+ << "removeAndDestroyObjectModelItemWithTransitions.qml" << OnlyDestroy;
+}
+
+// QTBUG-46798, QTBUG-133256
+void tst_QQuickListView2::removeAndDestroyObjectModelItem()
+{
+ QFETCH(QString, qmlFilePath);
+ QFETCH(RemovalPolicy, removalPolicy);
+
+ QQuickView window;
+ QVERIFY(QQuickTest::showView(window, testFileUrl(qmlFilePath)));
+
+ QQuickListView *listView = qobject_cast<QQuickListView*>(window.rootObject());
+ QVERIFY(listView);
+
+ auto *objectModel = listView->model().value<QQmlObjectModel *>();
+ QVERIFY(objectModel);
+ QPointer<QObject> firstItem = objectModel->get(0);
+ QVERIFY(firstItem);
+ // Shouldn't crash.
+ if (removalPolicy == RemoveAndDestroy)
+ objectModel->remove(0);
+ firstItem->deleteLater();
+ QTRY_VERIFY(!firstItem);
+
+ // Now try moving the view. It also shouldn't crash.
+ if (removalPolicy == RemoveAndDestroy) {
+ if (QQuickTest::qIsPolishScheduled(listView))
+ QVERIFY(QQuickTest::qWaitForPolish(listView));
+ }
+ listView->positionViewAtEnd();
+}
+
QTEST_MAIN(tst_QQuickListView2)
#include "tst_qquicklistview2.moc"
diff --git a/tests/auto/quickcontrols/controls/data/tst_container.qml b/tests/auto/quickcontrols/controls/data/tst_container.qml
index a829404a21..f6c50699e2 100644
--- a/tests/auto/quickcontrols/controls/data/tst_container.qml
+++ b/tests/auto/quickcontrols/controls/data/tst_container.qml
@@ -293,4 +293,78 @@ TestCase {
compare(control.implicitWidth, 100)
compare(control.implicitHeight, 30)
}
+
+ Transition {
+ id: yTransition
+
+ NumberAnimation {
+ properties: "y"
+ duration: 1000
+ }
+ }
+
+ Transition {
+ id: scaleTransition
+
+ NumberAnimation {
+ properties: "scale"
+ from: 1
+ to: 0
+ duration: 400
+ }
+ }
+
+ Component {
+ id: listViewContainerComponent
+
+ Container {
+ id: container
+ width: 200
+ height: 200
+
+ property alias addDisplaced: view.addDisplaced
+ property alias moveDisplaced: view.moveDisplaced
+ property alias removeDisplaced: view.removeDisplaced
+ property alias displaced: view.displaced
+ property alias remove: view.remove
+
+ contentItem: ListView {
+ id: view
+ objectName: "containerListView"
+ model: container.contentModel
+ spacing: 5
+ }
+ }
+ }
+
+ function test_listViewTransitions_data() {
+ return [
+ { tag: "addDisplaced", addDisplaced: yTransition },
+ { tag: "moveDisplaced", moveDisplaced: yTransition },
+ { tag: "removeDisplaced", removeDisplaced: yTransition },
+ { tag: "displaced", displaced: yTransition },
+ { tag: "remove", remove: scaleTransition }
+ ]
+ }
+
+ function test_listViewTransitions(data) {
+ let control = createTemporaryObject(listViewContainerComponent, testCase, {
+ addDisplaced: data.addDisplaced ?? null,
+ moveDisplaced: data.moveDisplaced ?? null,
+ removeDisplaced: data.removeDisplaced ?? null,
+ displaced: data.displaced ?? null,
+ remove: data.remove ?? null
+ })
+ verify(control)
+
+ let rect = rectangle.createObject(control,
+ { width: 200, height: 40, color: "tomato", objectName: "tomatoRect" })
+ control.addItem(rect)
+ verify(isPolishScheduled(testCase.Window.window))
+ verify(waitForPolish(testCase.Window.window))
+
+ control.removeItem(rect)
+ verify(isPolishScheduled(testCase.Window.window))
+ verify(waitForPolish(testCase.Window.window))
+ }
}