diff options
author | Axel Spoerl <axel.spoerl@qt.io> | 2023-07-05 10:24:35 +0200 |
---|---|---|
committer | Axel Spoerl <axel.spoerl@qt.io> | 2023-08-15 10:04:08 +0000 |
commit | 199fb8b75562ff1d649efb910d05c165329fcba8 (patch) | |
tree | 5e2df67cddfb5ff93df294c4e12daaa9be4e0802 | |
parent | 1508354fa7e68c5dd8913e2628679dff28adc0c7 (diff) |
QQmlDelegateModelPrivate:object: release objects only with objectRef >0
If a cache item has an object and there are no pending incubation
tasks, it returns the object.
Otherwise, it releases the object, before deleting the cache item, if
it is unreferenced.
If the cache item's object has a zero objectRef, because it is still
under construction, the method attempts to release it.
Releasing an object with a zero objectRef triggers an assertion in
QQmlDelegateModelItem::releaseObject().
The cacheItem's object is referenced in qqmldelegatemodel.cpp:1234,
which increases the objectRef from 0 to 1. It is set to 0 again,
if the cacheItem has a delegate, which gets incubated in
qqmldelegatemodel.cpp:1281. This case is not reflected later, and the
cacheItem's object is unconditionally released.
This patch makes the release attempt conditional to a positive
objectRef. It adds an autotest in tst_QQmlDelegateModel.
Fixes: QTBUG-104469
Change-Id: I5c0751ca7f796a9701889520c526f719a27a200b
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
(cherry picked from commit e9f631b09265f575575038f6979443143e2121c0)
3 files changed, 32 insertions, 1 deletions
diff --git a/src/qmlmodels/qqmldelegatemodel.cpp b/src/qmlmodels/qqmldelegatemodel.cpp index 3aa7752632..581c620361 100644 --- a/src/qmlmodels/qqmldelegatemodel.cpp +++ b/src/qmlmodels/qqmldelegatemodel.cpp @@ -1298,7 +1298,9 @@ QObject *QQmlDelegateModelPrivate::object(Compositor::Group group, int index, QQ if (cacheItem->object && (!cacheItem->incubationTask || isDoneIncubating(cacheItem->incubationTask->status()))) return cacheItem->object; - cacheItem->releaseObject(); + if (cacheItem->objectRef > 0) + cacheItem->releaseObject(); + if (!cacheItem->isReferenced()) { removeCacheItem(cacheItem); delete cacheItem; diff --git a/tests/auto/qml/qqmldelegatemodel/data/modifyObjectUnderConstruction.qml b/tests/auto/qml/qqmldelegatemodel/data/modifyObjectUnderConstruction.qml new file mode 100644 index 0000000000..f21577e395 --- /dev/null +++ b/tests/auto/qml/qqmldelegatemodel/data/modifyObjectUnderConstruction.qml @@ -0,0 +1,18 @@ +import QtQuick + +Window { + id: window + width: 640 + height: 480 + visible: true + property alias testModel: repeater.model + + Repeater { + id: repeater + model: 1 + delegate: Item { + Component.onCompleted: repeater.model = 0 + } + } +} + diff --git a/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp b/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp index 38f2a8c212..c8f6c39b18 100644 --- a/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp +++ b/tests/auto/qml/qqmldelegatemodel/tst_qqmldelegatemodel.cpp @@ -33,6 +33,7 @@ private slots: void typedModelData(); void deleteRace(); void persistedItemsStayInCache(); + void doNotUnrefObjectUnderConstruction(); }; class AbstractItemModel : public QAbstractItemModel @@ -474,6 +475,16 @@ void tst_QQmlDelegateModel::persistedItemsStayInCache() QTRY_COMPARE(object->property("destroyCount").toInt(), 3); } +void tst_QQmlDelegateModel::doNotUnrefObjectUnderConstruction() +{ + QQmlEngine engine; + QQmlComponent component(&engine, testFileUrl("modifyObjectUnderConstruction.qml")); + QVERIFY2(component.isReady(), qPrintable(component.errorString())); + std::unique_ptr<QObject> object(component.create()); + QVERIFY(object); + QTRY_COMPARE(object->property("testModel").toInt(), 0); +} + QTEST_MAIN(tst_QQmlDelegateModel) #include "tst_qqmldelegatemodel.moc" |