From db6f1440cbe78018e442c1fb961310a4e619e8fe Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 9 Jun 2017 23:36:49 +0200 Subject: QQuickItemView: fix releaseItem() loops Calling releaseItem() destroys the item, which emits childrenChanged for the contentItem, and if at that point anything calls setFooMargin(), setContentHeight(), returnToBounds(), or many other methods that indirectly access the visibleItems list, it leads to a crash due to read after free. Add a releaseVisibleItems() helper method that makes a copy, clears the original list first, and then releases the items. Task-number: QTBUG-48394 Task-number: QTBUG-61294 Change-Id: I29e4d3870d33549e8bf789de84c67ab1826fca7d Reviewed-by: Robin Burchell --- src/quick/items/qquickgridview.cpp | 4 +--- src/quick/items/qquickitemview.cpp | 8 ++------ src/quick/items/qquickitemview_p_p.h | 9 +++++++++ src/quick/items/qquicklistview.cpp | 4 +--- 4 files changed, 13 insertions(+), 12 deletions(-) (limited to 'src/quick') diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp index fd78c46a16..c570b95a21 100644 --- a/src/quick/items/qquickgridview.cpp +++ b/src/quick/items/qquickgridview.cpp @@ -495,9 +495,7 @@ bool QQuickGridViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, qreal // We've jumped more than a page. Estimate which items are now // visible and fill from there. int count = (fillFrom - (rowPos + rowSize())) / (rowSize()) * columns; - for (FxViewItem *item : qAsConst(visibleItems)) - releaseItem(item); - visibleItems.clear(); + releaseVisibleItems(); modelIndex += count; if (modelIndex >= model->count()) modelIndex = model->count() - 1; diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp index 555db03962..084b1f197a 100644 --- a/src/quick/items/qquickitemview.cpp +++ b/src/quick/items/qquickitemview.cpp @@ -380,9 +380,7 @@ void QQuickItemView::setDelegate(QQmlComponent *delegate) int oldCount = dataModel->count(); dataModel->setDelegate(delegate); if (isComponentComplete()) { - for (FxViewItem *item : qAsConst(d->visibleItems)) - d->releaseItem(item); - d->visibleItems.clear(); + d->releaseVisibleItems(); d->releaseItem(d->currentItem); d->currentItem = 0; d->updateSectionCriteria(); @@ -1743,9 +1741,7 @@ void QQuickItemViewPrivate::clear() currentChanges.reset(); timeline.clear(); - for (FxViewItem *item : qAsConst(visibleItems)) - releaseItem(item); - visibleItems.clear(); + releaseVisibleItems(); visibleIndex = 0; for (FxViewItem *item : qAsConst(releasePendingTransition)) { diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h index 3087682ac7..b6353246e8 100644 --- a/src/quick/items/qquickitemview_p_p.h +++ b/src/quick/items/qquickitemview_p_p.h @@ -269,6 +269,15 @@ public: q->polish(); } + void releaseVisibleItems() { + // make a copy and clear the visibleItems first to avoid destroyed + // items being accessed during the loop (QTBUG-61294) + const QList oldVisible = visibleItems; + visibleItems.clear(); + for (FxViewItem *item : oldVisible) + releaseItem(item); + } + QPointer model; QVariant modelVariant; int itemCount; diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp index f739115e6b..18f9b8512d 100644 --- a/src/quick/items/qquicklistview.cpp +++ b/src/quick/items/qquicklistview.cpp @@ -660,9 +660,7 @@ bool QQuickListViewPrivate::addVisibleItems(qreal fillFrom, qreal fillTo, qreal int newModelIdx = qBound(0, modelIndex + count, model->count()); count = newModelIdx - modelIndex; if (count) { - for (FxViewItem *item : qAsConst(visibleItems)) - releaseItem(item); - visibleItems.clear(); + releaseVisibleItems(); modelIndex = newModelIdx; visibleIndex = modelIndex; visiblePos = itemEnd + count * (averageSize + spacing); -- cgit v1.2.3