diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2022-03-17 15:23:55 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2022-03-19 07:53:39 +0000 |
commit | eb52c31c5b8cb6f05fce598519034c4fc32c4c3f (patch) | |
tree | 111328b7bf72942ebcebeffd5daf79421d37e051 | |
parent | b7986a8f6e9df3727f433a0df0ba56a3355153d0 (diff) |
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
Change-Id: I6da6f4043fe1906b0186931a37283f635cb5a404
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 5e566ab373bd6161f82b00c5a1f24bd210051140)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/widgets/graphicsview/qgraphicsscene.cpp | 6 | ||||
-rw-r--r-- | tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp | 69 |
2 files changed, 73 insertions, 2 deletions
diff --git a/src/widgets/graphicsview/qgraphicsscene.cpp b/src/widgets/graphicsview/qgraphicsscene.cpp index f41ced1a38..d00d69c47c 100644 --- a/src/widgets/graphicsview/qgraphicsscene.cpp +++ b/src/widgets/graphicsview/qgraphicsscene.cpp @@ -2285,9 +2285,11 @@ void QGraphicsScene::clearSelection() // Disable emitting selectionChanged ++d->selectionChanging; - bool changed = !d->selectedItems.isEmpty(); + // iterate over a copy, as clearing selection might invalidate selectedItems + const auto selectedItems = d->selectedItems; + bool changed = !selectedItems.isEmpty(); - for (QGraphicsItem *item : qAsConst(d->selectedItems)) + for (QGraphicsItem *item : selectedItems) item->setSelected(false); d->selectedItems.clear(); diff --git a/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/widgets/graphicsview/qgraphicsscene/tst_qgraphicsscene.cpp index b8991b5a9a..9a8b94852d 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; @@ -4969,5 +4971,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" |