diff options
author | Yulong Bai <yulong.bai@qt.io> | 2019-05-21 13:48:52 +0200 |
---|---|---|
committer | Yulong Bai <yulong.bai@qt.io> | 2019-06-17 16:19:15 +0200 |
commit | e9520ec84c95e10a6826b2289e46552a2d446895 (patch) | |
tree | 74b0a97cb50c3608f82b4f3bb2cc00e115164b73 /tests/auto/quick/qquickanimations | |
parent | c1663865e68d96d4a51351d4d1d2bfa5f313dc18 (diff) |
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 <richard.gustavsen@qt.io>
Diffstat (limited to 'tests/auto/quick/qquickanimations')
3 files changed, 130 insertions, 0 deletions
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<QQuickWindow> win(qobject_cast<QQuickWindow*>(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<QQmlTimer*>(); + QVERIFY(timer); + QCOMPARE(timer->isRunning(), false); + timer->start(); + QTest::qWait(1000); +} + QTEST_MAIN(tst_qquickanimations) #include "tst_qquickanimations.moc" |