From e9520ec84c95e10a6826b2289e46552a2d446895 Mon Sep 17 00:00:00 2001 From: Yulong Bai Date: Tue, 21 May 2019 13:48:52 +0200 Subject: Fix crash caused by objects self-destructions during displacement animations The root cause was that the QAbstractAnimationJob::finished() might delegate its destruction to change.listener->animationFinished(this), and the original author was aware of that and provided a RETURN_IF_DELETE macro to return early if itself got deleted. In the bug's case, change.listener->animationFinished(this) dispatched to QQuickItemViewPrivate::animationFinished() which called QQuickItemViewPrivate::release() and deleted the QAbstractAnimationJob object itself in the end. However, any objects derived from QAbstractAnimationJob, or holding a pointer to a QAbstractAnimationJob, may potentially fall into the code path calling QAbstractAnimationJob::finished(). Any QAnimationJobChangeListener that directly or indirectly deletes QAbstractAnimationJob should be very suspicious to this kind of "heap-use-after-free" bug. Should ensure that the QAbstractAnimationJob won't be referenced after deletion. In the bug's case, within the code path triggered by ListView displacement animation, the other affected classes by QAbstractAnimationJob are: QQuickItemViewFxItem, QQuickItemViewTransitionableItem, QQuickTransitionManager. To fix this, a new SelfDeletable class is factored out to simplify the self-deletion test logic. Any affected classes are made to have a public member m_selfDeletable. Any code paths that finally reach QAbstractAnimationJob::finished() are wrapped with related util macro. Change-Id: Idd33fc3f2d529fd7d8bb088c329101b1e70dd6c0 Task-number: QTBUG-44308 Reviewed-by: Richard Moe Gustavsen --- .../data/animationJobSelfDestructionBug.qml | 108 +++++++++++++++++++++ .../quick/qquickanimations/qquickanimations.pro | 1 + .../qquickanimations/tst_qquickanimations.cpp | 21 ++++ 3 files changed, 130 insertions(+) create mode 100644 tests/auto/quick/qquickanimations/data/animationJobSelfDestructionBug.qml (limited to 'tests/auto/quick/qquickanimations') diff --git a/tests/auto/quick/qquickanimations/data/animationJobSelfDestructionBug.qml b/tests/auto/quick/qquickanimations/data/animationJobSelfDestructionBug.qml new file mode 100644 index 0000000000..259871785b --- /dev/null +++ b/tests/auto/quick/qquickanimations/data/animationJobSelfDestructionBug.qml @@ -0,0 +1,108 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtCore module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** 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. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ +import QtQuick 2.11 +import QtQuick.Window 2.11 + +Window { + id: root + property alias timer : timer + property variant ops: [{'op': 'add', 'count': 3}, {'op': 'add', 'count': 6}, {'op': 'rem', 'count': 4}, {'op': 'rem', 'count': 1}, {'op': 'rem', 'count': 3}] + property int opIndex : 0 + width: 400 + height: 600 + + ListModel { + id: theModel + } + + Timer { + id: timer + interval: 100 + running: false + repeat: true + onTriggered: { + if (opIndex >= ops.length) { + timer.stop() + return + } + let op = ops[opIndex] + for (var i = 0; i < op.count; ++i) { + if (op.op === "add") + theModel.append({"name": "opIndex " + opIndex}) + else + theModel.remove(0, 1); + } + opIndex = opIndex + 1 + } + } + + ListView { + anchors.top: parent.top + anchors.right: parent.right + height: 600 + anchors.left: parent.horizontalCenter + spacing: 4 + model: theModel + header: Text { + text: "YAnimator" + } + add: Transition { + NumberAnimation { property: "scale"; from: 0; to: 1; duration: 200 } + NumberAnimation { property: "opacity"; from: 0; to: 1; duration: 200 } + } + displaced: Transition { + YAnimator { duration: 500 } + NumberAnimation { property: "opacity"; to: 1.0; duration: 500 } + NumberAnimation { property: "scale"; to: 1.0; duration: 500 } + } + remove: Transition { + NumberAnimation { property: "opacity"; to: 0; duration: 200 } + NumberAnimation { property: "scale"; to: 0; duration: 200 } + } + delegate: Rectangle { + width: 200 + height: 20 + color:"red" + Text { + anchors.centerIn: parent + text: name + } + } + } +} diff --git a/tests/auto/quick/qquickanimations/qquickanimations.pro b/tests/auto/quick/qquickanimations/qquickanimations.pro index 8bb1f47af5..cf9c87a305 100644 --- a/tests/auto/quick/qquickanimations/qquickanimations.pro +++ b/tests/auto/quick/qquickanimations/qquickanimations.pro @@ -12,6 +12,7 @@ QT += core-private gui-private qml-private quick-private testlib DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0 OTHER_FILES += \ + data/animationJobSelfDestructionBug.qml\ data/attached.qml \ data/badproperty1.qml \ data/badproperty2.qml \ diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp index 0f095774e8..1dad0c771c 100644 --- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp +++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp @@ -109,6 +109,7 @@ private slots: void unsetAnimatorProxyJobWindow(); void finished(); void replacingTransitions(); + void animationJobSelfDestruction(); }; #define QTIMED_COMPARE(lhs, rhs) do { \ @@ -1723,6 +1724,26 @@ void tst_qquickanimations::replacingTransitions() QCOMPARE(model->count(), 3); } +void tst_qquickanimations::animationJobSelfDestruction() +{ + // Don't crash + QQmlEngine engine; + engine.clearComponentCache(); + QQmlComponent c(&engine, testFileUrl("animationJobSelfDestructionBug.qml")); + QScopedPointer win(qobject_cast(c.create())); + if (!c.errors().isEmpty()) + qDebug() << c.errorString(); + QVERIFY(win); + win->setTitle(QTest::currentTestFunction()); + win->show(); + QVERIFY(QTest::qWaitForWindowExposed(win.data())); + QQmlTimer *timer = win->property("timer").value(); + QVERIFY(timer); + QCOMPARE(timer->isRunning(), false); + timer->start(); + QTest::qWait(1000); +} + QTEST_MAIN(tst_qquickanimations) #include "tst_qquickanimations.moc" -- cgit v1.2.3