Fix crash when QQuickWindow::updatePolish() deletes QQuickItems.

When an item is deleted as a result of an updatePolish() call, it will
be removed from the itemsToPolish set. However, updatePolish()
internally works on a copy of the itemsToPolish set because changes to
the set would invalidate its iterator. Because it is a copy will still
contain the deleted item.

Fix this by simplifying the algorithm to instead pick items one by
one from the itemsToPolish set until it is empty. The recursion guard
has been increased because we're not decrementing it for every single
QQuickItem::updatePolish() call.

Task-number: QTBUG-42913
Change-Id: If7ab7f7616b01daf4d3ed843f927c163dfb03843
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>

1 files changed, 16 insertions, 14 deletions

diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp
index 129c07da1d..f21a93b83c 100644
--- a/src/quick/items/qquickwindow.cpp
+++ b/src/quick/items/qquickwindow.cpp
@@ -249,20 +249,22 @@ void QQuickWindow::focusInEvent(QFocusEvent *ev)
 
 void QQuickWindowPrivate::polishItems()
 {
-    int maxPolishCycles = 100000;
-
-    while (!itemsToPolish.isEmpty() && --maxPolishCycles > 0) {
-        QSet<QQuickItem *> itms = itemsToPolish;
-        itemsToPolish.clear();
-
-        for (QSet<QQuickItem *>::iterator it = itms.begin(); it != itms.end(); ++it) {
-            QQuickItem *item = *it;
-            QQuickItemPrivate::get(item)->polishScheduled = false;
-            item->updatePolish();
-        }
-    }
-
-    if (maxPolishCycles == 0)
+    // An item can trigger polish on another item, or itself for that matter,
+    // during its updatePolish() call. Because of this, we cannot simply
+    // iterate through the set, we must continue pulling items out until it
+    // is empty.
+    // In the case where polish is called from updatePolish() either directly
+    // or indirectly, we use a recursionSafeguard to print a warning to
+    // the user.
+    int recursionSafeguard = INT_MAX;
+    while (!itemsToPolish.isEmpty() && --recursionSafeguard > 0) {
+        QQuickItem *item = *itemsToPolish.begin();
+        itemsToPolish.remove(item);
+        QQuickItemPrivate::get(item)->polishScheduled = false;
+        item->updatePolish();
+    }
+
+    if (recursionSafeguard == 0)
         qWarning("QQuickWindow: possible QQuickItem::polish() loop");
 
     updateFocusItemTransform();