QQuickPathViewPrivate: fix heap-use-after-free

The TabBar auto tests in Qt Quick Controls 2 repeats the following
process very quickly for several data rows:

1. Creates a TabBar (PathView, when using the Universal style)
2. Moves items in its QQmlObjectModel
3. Deletes the TabBar

When run with ASAN, this test would fail, because the TabButtons
(which are child items of the PathView) would try to access a deleted
QQuickItemChangeListener upon their destruction.

The underlying issue is that QQuickPathView::modelUpdated() is called,
and before a refill() can happen, the view is deleted.
QQuickPathView::refill() was the only execution path that was releasing
the cached items (QQuickPathViewPrivate::itemCache), and since part of
releasing an item involves removing the QQuickPathView as a change
listener from the item, the item would access the deleted view
(listener) when the item was being destroyed.

This patch fixes the issue by also releasing cached items in
QQuickPathViewPrivate::clear(), which is always called by the
destructor.

Task-number: QTBUG-68964
Change-Id: Ic5bf0943be79948c86bf7c07ef13ecd1a7b971ba
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Reviewed-by: Robin Burchell <robin.burchell@crimson.no>

2 files changed, 171 insertions, 0 deletions

diff --git a/tests/auto/quick/qquickpathview/data/objectModelMove.qml b/tests/auto/quick/qquickpathview/data/objectModelMove.qml
new file mode 100644
index 0000000000..d5fa510d69
--- /dev/null
+++ b/tests/auto/quick/qquickpathview/data/objectModelMove.qml
@@ -0,0 +1,123 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:BSD$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** BSD License Usage
+** Alternatively, you may use this file under the terms of the BSD license
+** as follows:
+**
+** "Redistribution and use in source and binary forms, with or without
+** modification, are permitted provided that the following conditions are
+** met:
+**   * Redistributions of source code must retain the above copyright
+**     notice, this list of conditions and the following disclaimer.
+**   * Redistributions in binary form must reproduce the above copyright
+**     notice, this list of conditions and the following disclaimer in
+**     the documentation and/or other materials provided with the
+**     distribution.
+**   * Neither the name of The Qt Company Ltd nor the names of its
+**     contributors may be used to endorse or promote products derived
+**     from this software without specific prior written permission.
+**
+**
+** THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+** "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+** LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+** A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+** OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+** SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+** LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+** OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE."
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQml.Models 2.11
+import QtQuick 2.11
+
+Item {
+    id: root
+    width: 400
+    height: 400
+    visible: true
+
+    property Item pathViewItem
+
+    function destroyView() {
+        if (pathViewItem)
+            pathViewItem.destroy()
+    }
+
+    function newView() {
+        pathViewItem = pathViewComponent.createObject(root)
+    }
+
+    function move() {
+        objectModel.move(0, 1)
+    }
+
+    Component {
+        id: pathViewComponent
+
+        PathView {
+            id: pathView
+            objectName: "PathView"
+            width: 32 * 3
+            height: 32
+            model: objectModel
+
+            interactive: false
+            snapMode: PathView.SnapToItem
+            movementDirection: PathView.Positive
+            highlightMoveDuration: 100
+
+            path: Path {
+                startX: 16
+                startY: 16
+                PathLine {
+                    x: 16 + (32 * 3)
+                    y: 16
+                }
+            }
+        }
+    }
+
+    ObjectModel {
+        id: objectModel
+
+        Rectangle {
+            objectName: "red"
+            width: 32
+            height: 32
+            color: "red"
+        }
+        Rectangle {
+            objectName: "green"
+            width: 32
+            height: 32
+            color: "green"
+        }
+        Rectangle {
+            objectName: "blue"
+            width: 32
+            height: 32
+            color: "blue"
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
index 4211d08393..e6c03d052c 100644
--- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
+++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
@@ -144,6 +144,7 @@ private slots:
     void movementDirection_data();
     void movementDirection();
     void removePath();
+    void objectModelMove();
 };
 
 class TestObject : public QObject
@@ -2522,6 +2523,53 @@ void tst_QQuickPathView::removePath()
     QVERIFY(QMetaObject::invokeMethod(pathview, "setPath"));
 }
 
+/*
+    Tests that moving items in an ObjectModel and then deleting the view
+    doesn't cause heap-use-after-free when run through ASAN.
+
+    The test case is based on a Qt Quick Controls 2 test where the issue was
+    discovered.
+*/
+void tst_QQuickPathView::objectModelMove()
+{
+    QScopedPointer<QQuickView> window(createView());
+    window->setSource(testFileUrl("objectModelMove.qml"));
+    window->show();
+
+    // Create the view.
+    QVERIFY(QMetaObject::invokeMethod(window->rootObject(), "newView"));
+    QPointer<QQuickPathView> pathView = window->rootObject()->property("pathViewItem").value<QQuickPathView*>();
+    QVERIFY(pathView);
+    QCOMPARE(pathView->count(), 3);
+    pathView->highlightItem()->setObjectName("highlight");
+
+    // Move an item from index 0 to 1.
+    QVERIFY(QMetaObject::invokeMethod(window->rootObject(), "move"));
+    QCOMPARE(pathView->count(), 3);
+
+    // Keep track of the amount of listeners
+    QVector<QString> itemObjectNames;
+    itemObjectNames << QLatin1String("red") << QLatin1String("green") << QLatin1String("blue");
+    QVector<QQuickItem*> childItems;
+    for (const QString itemObjectName : qAsConst(itemObjectNames)) {
+        QQuickItem *childItem = findItem<QQuickItem>(pathView, itemObjectName);
+        QVERIFY(childItem);
+        childItems.append(childItem);
+    }
+
+    // Destroy the view (via destroy()).
+    QVERIFY(QMetaObject::invokeMethod(window->rootObject(), "destroyView"));
+    // Ensure that the view has been destroyed. This check is also necessary in order for
+    // ASAN to complain (it will complain after the test function has finished).
+    QTRY_VERIFY(pathView.isNull());
+    // By this point, all of its cached items should have been released,
+    // which means none of the items should have any listeners.
+    for (const auto childItem : qAsConst(childItems)) {
+        const QQuickItemPrivate *childItemPrivate = QQuickItemPrivate::get(childItem);
+        QCOMPARE(childItemPrivate->changeListeners.size(), 0);
+    }
+}
+
 QTEST_MAIN(tst_QQuickPathView)
 
 #include "tst_qquickpathview.moc"