Avoid crashes when invalidating a proxy model filter.

Task-number: QTBUG-26107
Change-Id: I2df7ae6402136570c8469d3251edae6ca8290f1f
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>

diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
index 875bb81..fb84dce 100644 (file)
--- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp
+++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp
@@ -1049,12 +1049,19 @@ void QSortFilterProxyModelPrivate::filter_changed(const QModelIndex &source_pare
     Mapping *m = it.value();
     QSet<int> rows_removed = handle_filter_changed(m->proxy_rows, m->source_rows, source_parent, Qt::Vertical);
     QSet<int> columns_removed = handle_filter_changed(m->proxy_columns, m->source_columns, source_parent, Qt::Horizontal);
-    QVector<QModelIndex>::iterator it2 = m->mapped_children.end();
-    while (it2 != m->mapped_children.begin()) {
+
+    // We need to iterate over a copy of m->mapped_children because otherwise it may be changed by other code, invalidating
+    // the iterator it2.
+    // The m->mapped_children vector can be appended to when this function recurses for child indexes.
+    // The handle_filter_changed implementation can cause source_parent.parent() to be called, which will create
+    // a mapping (and do appending) while we are invalidating the filter.
+    QVector<QModelIndex> mappedChildren = m->mapped_children;
+    QVector<QModelIndex>::iterator it2 = mappedChildren.end();
+    while (it2 != mappedChildren.begin()) {
         --it2;
         const QModelIndex source_child_index = *it2;
         if (rows_removed.contains(source_child_index.row()) || columns_removed.contains(source_child_index.column())) {
-            it2 = m->mapped_children.erase(it2);
+            it2 = mappedChildren.erase(it2);
             remove_from_mapping(source_child_index);
         } else {
             filter_changed(source_child_index);

diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index bb3132b..3770abb 100644 (file)
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -144,6 +144,8 @@ private slots:
     void testParentLayoutChanged();
     void moveSourceRows();
 
+    void hierarchyFilterInvalidation();
+
 protected:
     void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
     void checkHierarchy(const QStringList &data, const QAbstractItemModel *model);
@@ -3480,5 +3482,71 @@ void tst_QSortFilterProxyModel::moveSourceRows()
     QCOMPARE(filterBothAfterParentLayoutSpy.size(), 0);
 }
 
+class FilterProxy : public QSortFilterProxyModel
+{
+    Q_OBJECT
+public:
+    FilterProxy(QObject *parent = 0)
+      : QSortFilterProxyModel(parent),
+        mode(false)
+    {
+
+    }
+
+public slots:
+    void setMode(bool on)
+    {
+        mode = on;
+        invalidateFilter();
+    }
+
+protected:
+    virtual bool filterAcceptsRow ( int source_row, const QModelIndex & source_parent ) const
+    {
+        if (mode) {
+            if (!source_parent.isValid()) {
+                return true;
+            } else {
+                return (source_row % 2) != 0;
+            }
+        } else {
+            if (!source_parent.isValid()) {
+                return source_row >= 2 && source_row < 10;
+            } else {
+                return true;
+            }
+        }
+    }
+
+private:
+    bool mode;
+};
+
+void tst_QSortFilterProxyModel::hierarchyFilterInvalidation()
+{
+    QStandardItemModel model;
+    for (int i = 0; i < 10; ++i) {
+        QStandardItem *child = new QStandardItem(QString("Row %1").arg(i));
+        for (int j = 0; j < 1; ++j) {
+            child->appendRow(new QStandardItem(QString("Row %1/%2").arg(i).arg(j)));
+        }
+        model.appendRow(child);
+    }
+
+    FilterProxy proxy;
+    proxy.setSourceModel(&model);
+
+    QTreeView view;
+    view.setModel(&proxy);
+
+    view.setCurrentIndex(proxy.index(2, 0).child(0, 0));
+
+    view.show();
+    QTest::qWaitForWindowExposed(&view);
+
+    proxy.setMode(true);
+}
+
+
 QTEST_MAIN(tst_QSortFilterProxyModel)
 #include "tst_qsortfilterproxymodel.moc"