When clearing selected items, iterate over a copy of the QSet

Selection change handlers of the items might call a method that
implicitly recreates the selectedItems QSet, which then invalidates
the iterators of the ranged for loop, resulting in crashes.

Iterate over a copy of the set instead.

Add a test case that crashen without the fix.

Fixes: QTBUG-101651
Pick-to: 6.3 6.2
Change-Id: I6da6f4043fe1906b0186931a37283f635cb5a404
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

1 files changed, 69 insertions, 0 deletions

diff --git a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp
index ba831c292d..d8d49d6b07 100644
--- a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp
+++ b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp
@@ -294,6 +294,8 @@ private slots:
     void taskQTBUG_42915_focusNextPrevChild();
     void taskQTBUG_85088_previewTextfailWhenLostFocus();
 
+    void deleteItemsOnChange();
+
 private:
     QRect m_availableGeometry = QGuiApplication::primaryScreen()->availableGeometry();
     QSize m_testSize;
@@ -4970,5 +4972,72 @@ void tst_QGraphicsScene::taskQTBUG_85088_previewTextfailWhenLostFocus()
     QCOMPARE(simpleTextItem->toPlainText(), str + str);
 }
 
+void tst_QGraphicsScene::deleteItemsOnChange()
+{
+    QGraphicsScene scene;
+
+    class SelectionItem : public QGraphicsRectItem {
+    public:
+        QRectF boundingRect() const override { return QRectF(); }
+    };
+
+    class ChangeItem : public QGraphicsItem
+    {
+    public:
+        ChangeItem()
+        {
+            setFlag(QGraphicsItem::ItemIsSelectable, true);
+            setFlag(QGraphicsItem::ItemIsMovable, true);
+        }
+        QRectF boundingRect() const override
+        {
+            return QRectF(0,0,100,100);
+        }
+
+    protected:
+        void paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *) override
+        {
+            painter->fillRect(boundingRect().toRect(), isSelected() ? Qt::yellow : Qt::cyan);
+        }
+
+        QVariant itemChange(GraphicsItemChange change, const QVariant &value) override
+        {
+            if (change != QGraphicsItem::ItemSelectedHasChanged)
+                return QGraphicsItem::itemChange(change, value);
+            if (value.toBool()) {
+                selectionRect = new SelectionItem;
+                scene()->addItem(selectionRect);
+            } else {
+                // this recreates the selectedItems QSet inside of QGraphicsScene,
+                // invalidating iterators. See QTBUG-101651.
+                scene()->selectedItems();
+                delete selectionRect;
+                selectionRect = nullptr;
+            }
+            return QGraphicsItem::itemChange(change, value);
+        }
+    private:
+        SelectionItem *selectionRect = nullptr;
+    };
+
+    ChangeItem item1;
+    item1.setPos(0, 0);
+    ChangeItem item2;
+    item1.setPos(50, 50);
+
+    scene.addItem(&item1);
+    scene.addItem(&item2);
+
+    QGraphicsView view;
+    view.setScene(&scene);
+    view.show();
+
+    QVERIFY(QTest::qWaitForWindowExposed(&view));
+
+    // this should not crash - see QTBUG-101651
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, {}, QPoint(120, 120));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, {}, QPoint(25, 25));
+}
+
 QTEST_MAIN(tst_QGraphicsScene)
 #include "tst_qgraphicsscene.moc"