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>
 

1 files changed, 21 insertions, 0 deletions

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"