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 --- .../quick/qquickanimations/tst_qquickanimations.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'tests/auto/quick/qquickanimations/tst_qquickanimations.cpp') 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 From b8a85408d943bffba403d783b9082bd279460bed Mon Sep 17 00:00:00 2001 From: Yulong Bai Date: Wed, 19 Jun 2019 15:05:28 +0200 Subject: QQuickItemView: fix crash while doing fast flicking in transitions The cause was that fast flicking kicked items in and out of viewport, while in transition, they would abruptly having tracking data structure , i.e. releasePendingTransition of QQuickItemViewPrivate, got iterator invalidated. This also helps to resolve QTBUG-44308. Fixes: QTBUG-76433 Fixes: QTBUG-44308 Change-Id: If14533d3f6b1acd7b6ca0c5c723347c0cb3f54dc Reviewed-by: Frederik Gladhorn --- .../qquickanimations/tst_qquickanimations.cpp | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'tests/auto/quick/qquickanimations/tst_qquickanimations.cpp') diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp index 1dad0c771c..d9b42bdeb5 100644 --- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp +++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include @@ -110,6 +111,7 @@ private slots: void finished(); void replacingTransitions(); void animationJobSelfDestruction(); + void fastFlickingBug(); }; #define QTIMED_COMPARE(lhs, rhs) do { \ @@ -1744,6 +1746,34 @@ void tst_qquickanimations::animationJobSelfDestruction() QTest::qWait(1000); } +void tst_qquickanimations::fastFlickingBug() +{ + // Don't crash + QQmlEngine engine; + engine.clearComponentCache(); + QQmlComponent c(&engine, testFileUrl("fastFlickingBug.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())); + auto timer = win->property("timer").value(); + QVERIFY(timer); + QCOMPARE(timer->isRunning(), false); + auto listView = win->property("listView").value(); + QVERIFY(listView); + timer->start(); + // flick listView up and down quickly in the middle of a slow transition + for (int sign = 1; timer->isRunning(); sign *= -1) { + listView->flick(0, sign * 4000); + qApp->processEvents(); + QTest::qWait(53); + qApp->processEvents(); + } +} + QTEST_MAIN(tst_qquickanimations) #include "tst_qquickanimations.moc" -- cgit v1.2.3