diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/qml/types/qqmldelegatemodel.cpp | 7 | ||||
-rw-r--r-- | src/qml/types/qqmllistmodel.cpp | 25 | ||||
-rw-r--r-- | src/qml/types/qqmllistmodel_p_p.h | 2 |
3 files changed, 27 insertions, 7 deletions
diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp index 9b1417eccd..26e6a81418 100644 --- a/src/qml/types/qqmldelegatemodel.cpp +++ b/src/qml/types/qqmldelegatemodel.cpp @@ -1266,6 +1266,13 @@ void QQmlDelegateModel::_q_itemsInserted(int index, int count) d->emitChanges(); } +//### This method should be split in two. It will remove delegates, and it will re-render the list. +// When e.g. QQmlListModel::remove is called, the removal of the delegates should be done on +// QAbstractItemModel::rowsAboutToBeRemoved, and the re-rendering on +// QAbstractItemModel::rowsRemoved. Currently both are done on the latter signal. The problem is +// that the destruction of an item will emit a changed signal that ends up at the delegate, which +// in turn will try to load the data from the model (which should have already freed it), resulting +// in a use-after-free. See QTBUG-59256. void QQmlDelegateModelPrivate::itemsRemoved( const QVector<Compositor::Remove> &removes, QVarLengthArray<QVector<QQmlChangeSet::Change>, Compositor::MaximumGroupCount> *translatedRemoves, diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp index 9d0f1afb32..b33be2b4fb 100644 --- a/src/qml/types/qqmllistmodel.cpp +++ b/src/qml/types/qqmllistmodel.cpp @@ -567,14 +567,20 @@ void ListModel::clear() elements.clear(); } -void ListModel::remove(int index, int count) +QVector<std::function<void()>> ListModel::remove(int index, int count) { + QVector<std::function<void()>> toDestroy; + auto layout = m_layout; for (int i=0 ; i < count ; ++i) { - elements[index+i]->destroy(m_layout); - delete elements[index+i]; + auto element = elements[index+i]; + toDestroy.append([element, layout](){ + element->destroy(layout); + delete element; + }); } elements.remove(index, count); updateCacheIndices(); + return toDestroy; } void ListModel::insert(int elementIndex, QV4::Object *object) @@ -2053,15 +2059,22 @@ void QQmlListModel::remove(QQmlV4Function *args) emitItemsAboutToBeRemoved(index, removeCount); + QVector<std::function<void()>> toDestroy; if (m_dynamicRoles) { - for (int i=0 ; i < removeCount ; ++i) - delete m_modelObjects[index+i]; + for (int i=0 ; i < removeCount ; ++i) { + auto modelObject = m_modelObjects[index+i]; + toDestroy.append([modelObject](){ + delete modelObject; + }); + } m_modelObjects.remove(index, removeCount); } else { - m_listModel->remove(index, removeCount); + toDestroy = m_listModel->remove(index, removeCount); } emitItemsRemoved(index, removeCount); + for (const auto &destroyer : toDestroy) + destroyer(); } else { qmlWarning(this) << tr("remove: incorrect number of arguments"); } diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h index cdce78e542..4928ad3725 100644 --- a/src/qml/types/qqmllistmodel_p_p.h +++ b/src/qml/types/qqmllistmodel_p_p.h @@ -367,7 +367,7 @@ public: void insert(int elementIndex, QV4::Object *object); void clear(); - void remove(int index, int count); + Q_REQUIRED_RESULT QVector<std::function<void()>> remove(int index, int count); int appendElement(); void insertElement(int index); |