aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMitch Curtis <mitch.curtis@qt.io>2019-11-27 11:00:59 +0100
committerPasi Petäjäjärvi <pasi.petajajarvi@qt.io>2019-12-09 09:14:45 +0000
commit16c983f3eeb2cbc83e863a43cd288d788aedefdc (patch)
tree9ca53c70f38a418ec19b3fc7c63279f27e4be83d
parentc7c8ab7a74ec3370ff5022d27bb8137a52edd433 (diff)
StackView: fix crash when recursively removing items5.12
This can happen when e.g. calling clear() in Component.onDestruction in response to a pop() call. The patch fixes the crash by warning and returning early. If users really need to do this, the clear() call can be delayed: Component.onDestruction: { Qt.callLater(function() { stackView.clear(StackView.Immediate) }) } Change-Id: If3cf07495bb34b96089522f44c36976bd6c62492 Fixes: QTBUG-80353 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io> (cherry picked from commit aaec25a798352fc222f86ab3b299384575f51dc8) Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
-rw-r--r--src/quicktemplates2/qquickstackview.cpp20
-rw-r--r--src/quicktemplates2/qquickstackview_p_p.h1
-rw-r--r--tests/auto/controls/data/tst_stackview.qml100
3 files changed, 121 insertions, 0 deletions
diff --git a/src/quicktemplates2/qquickstackview.cpp b/src/quicktemplates2/qquickstackview.cpp
index 18f65127..16e5f08d 100644
--- a/src/quicktemplates2/qquickstackview.cpp
+++ b/src/quicktemplates2/qquickstackview.cpp
@@ -649,6 +649,13 @@ void QQuickStackView::push(QQmlV4Function *args)
void QQuickStackView::pop(QQmlV4Function *args)
{
Q_D(QQuickStackView);
+ if (d->removingElements) {
+ d->warn(QStringLiteral("cannot pop while already in the process of removing elements"));
+ args->setReturnValue(QV4::Encode::null());
+ return;
+ }
+
+ QScopedValueRollback<bool> removingElements(d->removingElements, true);
QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("pop"));
int argc = args->length();
if (d->elements.count() <= 1 || argc > 2) {
@@ -804,6 +811,13 @@ void QQuickStackView::pop(QQmlV4Function *args)
void QQuickStackView::replace(QQmlV4Function *args)
{
Q_D(QQuickStackView);
+ if (d->removingElements) {
+ d->warn(QStringLiteral("cannot replace while already in the process of removing elements"));
+ args->setReturnValue(QV4::Encode::null());
+ return;
+ }
+
+ QScopedValueRollback<bool> removingElements(d->removingElements, true);
QScopedValueRollback<QString> rollback(d->operation, QStringLiteral("replace"));
if (args->length() <= 0) {
d->warn(QStringLiteral("missing arguments"));
@@ -900,6 +914,12 @@ void QQuickStackView::clear(Operation operation)
if (d->elements.isEmpty())
return;
+ if (d->removingElements) {
+ d->warn(QStringLiteral("cannot clear while already in the process of removing elements"));
+ return;
+ }
+
+ QScopedValueRollback<bool> removingElements(d->removingElements, true);
if (operation != Immediate) {
QQuickStackElement *exit = d->elements.pop();
exit->removal = true;
diff --git a/src/quicktemplates2/qquickstackview_p_p.h b/src/quicktemplates2/qquickstackview_p_p.h
index 74ea3e7a..b687561c 100644
--- a/src/quicktemplates2/qquickstackview_p_p.h
+++ b/src/quicktemplates2/qquickstackview_p_p.h
@@ -93,6 +93,7 @@ public:
void depthChange(int newDepth, int oldDepth);
bool busy = false;
+ bool removingElements = false;
QString operation;
QJSValue initialItem;
QQuickItem *currentItem = nullptr;
diff --git a/tests/auto/controls/data/tst_stackview.qml b/tests/auto/controls/data/tst_stackview.qml
index a9fbf874..c6515f77 100644
--- a/tests/auto/controls/data/tst_stackview.qml
+++ b/tests/auto/controls/data/tst_stackview.qml
@@ -1245,4 +1245,104 @@ TestCase {
gc()
verify(control.initialItem)
}
+
+ // Need to use this specific structure in order to reproduce the crash.
+ Component {
+ id: clearUponDestructionContainerComponent
+
+ Item {
+ id: container
+ objectName: "container"
+
+ property alias control: stackView
+ property var onDestructionCallback
+
+ property Component clearUponDestructionComponent: Component {
+ id: clearUponDestructionComponent
+
+ Item {
+ objectName: "clearUponDestructionItem"
+ Component.onDestruction: container.onDestructionCallback(stackView)
+ }
+ }
+
+ StackView {
+ id: stackView
+ initialItem: Item {
+ objectName: "initialItem"
+ }
+ }
+ }
+ }
+
+ // QTBUG-80353
+ // Tests that calling clear() in Component.onDestruction in response to that
+ // item being removed (e.g. via an earlier call to clear()) results in a warning and not a crash.
+ function test_recursiveClearClear() {
+ let container = createTemporaryObject(clearUponDestructionContainerComponent, testCase,
+ { onDestructionCallback: function(stackView) { stackView.clear(StackView.Immediate) }})
+ verify(container)
+
+ let control = container.control
+ control.push(container.clearUponDestructionComponent, StackView.Immediate)
+
+ // Shouldn't crash.
+ ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+ control.clear(StackView.Immediate)
+ }
+
+ function test_recursivePopClear() {
+ let container = createTemporaryObject(clearUponDestructionContainerComponent, testCase,
+ { onDestructionCallback: function(stackView) { stackView.clear(StackView.Immediate) }})
+ verify(container)
+
+ let control = container.control
+ control.push(container.clearUponDestructionComponent, StackView.Immediate)
+
+ // Pop all items except the first, removing the second item we pushed in the process.
+ // Shouldn't crash.
+ ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+ control.pop(null, StackView.Immediate)
+ }
+
+ function test_recursivePopPop() {
+ let container = createTemporaryObject(clearUponDestructionContainerComponent, testCase,
+ { onDestructionCallback: function(stackView) { stackView.pop(null, StackView.Immediate) }})
+ verify(container)
+
+ let control = container.control
+ // Push an extra item so that we can call pop(null) and reproduce the conditions for the crash.
+ control.push(component, StackView.Immediate)
+ control.push(container.clearUponDestructionComponent, StackView.Immediate)
+
+ // Pop the top item, then pop down to the first item in response.
+ ignoreWarning(new RegExp(".*cannot pop while already in the process of removing elements"))
+ control.pop(StackView.Immediate)
+ }
+
+ function test_recursiveReplaceClear() {
+ let container = createTemporaryObject(clearUponDestructionContainerComponent, testCase,
+ { onDestructionCallback: function(stackView) { stackView.clear(StackView.Immediate) }})
+ verify(container)
+
+ let control = container.control
+ control.push(container.clearUponDestructionComponent, StackView.Immediate)
+
+ // Replace the top item, then clear in response.
+ ignoreWarning(new RegExp(".*cannot clear while already in the process of removing elements"))
+ control.replace(component, StackView.Immediate)
+ }
+
+ function test_recursiveClearReplace() {
+ let container = createTemporaryObject(clearUponDestructionContainerComponent, testCase,
+ { onDestructionCallback: function(stackView) { stackView.replace(component, StackView.Immediate) }})
+ verify(container)
+
+ let control = container.control
+ control.push(container.clearUponDestructionComponent, StackView.Immediate)
+
+ // Replace the top item, then clear in response.
+ ignoreWarning(new RegExp(".*cannot replace while already in the process of removing elements"))
+ control.clear(StackView.Immediate)
+ }
}