Unset the QQmlAnimationTimer pointer from unregistered jobs

Amends 4b125a5bfdd935d260118406c343535e76023200, which reset the pointer
in registered jobs when the timer was destroyed. However, if a job is
unregistered explicitly before the timer is destroyed, then the job's
m_timer pointer might still be a dangling pointer, resulting in a crash
when the job is reactivated later.

So clear the job's pointer to QQmlAnimationTimer whenever it is
unregistered from the timer. Since a running job can then have a nullptr
timer, only assert in the job's destructor if the timer is not nullptr.

Introduce a test case that reliably crashes without the fix. The test
is added to the QQuickAnimation test as it uses a Qt Quick UI.

Fixes: QTBUG-98248
Change-Id: Ief991900c50aefd480d9c79e83324968102ca29c
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 9c732286ed72afe8cc2d2062e6cedbe5f6a06447)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

3 files changed, 50 insertions, 2 deletions

diff --git a/src/qml/animations/qabstractanimationjob.cpp b/src/qml/animations/qabstractanimationjob.cpp
index 5e03480206..05cf786e7d 100644
--- a/src/qml/animations/qabstractanimationjob.cpp
+++ b/src/qml/animations/qabstractanimationjob.cpp
@@ -243,6 +243,7 @@ void QQmlAnimationTimer::registerRunningAnimation(QAbstractAnimationJob *animati
 
 void QQmlAnimationTimer::unregisterRunningAnimation(QAbstractAnimationJob *animation)
 {
+    unsetJobTimer(animation);
     if (animation->userControlDisabled())
         return;
 
@@ -307,9 +308,10 @@ QAbstractAnimationJob::~QAbstractAnimationJob()
 
         Q_ASSERT(m_state == Stopped);
         if (oldState == Running) {
-            Q_ASSERT(QQmlAnimationTimer::instance(false) == m_timer);
-            if (m_timer)
+            if (m_timer) {
+                Q_ASSERT(QQmlAnimationTimer::instance(false) == m_timer);
                 m_timer->unregisterAnimation(this);
+            }
         }
         Q_ASSERT(!m_hasRegisteredTimer);
     }
diff --git a/tests/auto/quick/qquickanimations/data/cleanupWhenRenderThreadStops.qml b/tests/auto/quick/qquickanimations/data/cleanupWhenRenderThreadStops.qml
new file mode 100644
index 0000000000..4cfb59aeab
--- /dev/null
+++ b/tests/auto/quick/qquickanimations/data/cleanupWhenRenderThreadStops.qml
@@ -0,0 +1,32 @@
+//main.qml
+
+import QtQuick 2.12
+import QtQuick.Window 2.12
+
+Item {
+    id: root
+    width: 640
+    height: 480
+    visible: true
+    property bool running : false
+
+    Rectangle {
+        id: rect
+        anchors.fill: parent
+        color: "red"
+
+        Component.onCompleted: {
+            anim.start()
+            running = true
+        }
+    }
+
+    OpacityAnimator {
+        id: anim
+
+        target: rect
+        from: 1
+        to: 0
+        duration: 20000
+    }
+}
diff --git a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
index f373f53f1b..e2bac5ee39 100644
--- a/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
+++ b/tests/auto/quick/qquickanimations/tst_qquickanimations.cpp
@@ -116,6 +116,7 @@ private slots:
     void fastFlickingBug();
     void opacityAnimationFromZero();
     void alwaysRunToEndInSequentialAnimationBug();
+    void cleanupWhenRenderThreadStops();
 };
 
 #define QTIMED_COMPARE(lhs, rhs) do { \
@@ -1998,6 +1999,19 @@ void tst_qquickanimations::alwaysRunToEndInSequentialAnimationBug()
     QCOMPARE(whiteRect->property("opacity").value<qreal>(),1.0);
 }
 
+void tst_qquickanimations::cleanupWhenRenderThreadStops()
+{
+    QQuickView view(QUrl::fromLocalFile("data/cleanupWhenRenderThreadStops.qml"));
+    view.show();
+    view.setPersistentGraphics(false);
+    view.setPersistentSceneGraph(false);
+    QVERIFY(QTest::qWaitForWindowExposed(&view));
+    QTest::qWait(50);
+    view.hide();
+    view.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&view));
+}
+
 QTEST_MAIN(tst_qquickanimations)
 
 #include "tst_qquickanimations.moc"