Reset the timer of all controlled animations upon timer destruction

Fixes a crash when quitting an application while an animation parented
within the regular item tree is running. This amends
0e797296f9bb590926803463c32681e7fcd46064, which only turned the crash
into a Q_ASSERT failure.

As the render thread exits, the QThreadStorage destroys the timer.
That left the m_timer of the QAbstractAnimationJob a dangling pointer,
so we need to iterate over all registered animtation, and reset that
pointer to nullptr.

The animation jobs will then be destroyed later as part of regular object
tree cleanup. They will set their state to Stopped, which attempted
to unregister from the timer, creating one if needed. This also crashed
as the QThread was already finished. Don't create the timer if all we
want to do is stop the animation anyway.

Since testing this requires control over the thread lifecycle, this fix
does not come with an auto test.

Change-Id: Ice0c83818dd712c9e3be1261b79631f639de61a0
(cherry picked from commit 66a5167dab35aedf1c7bef3e0f8deea903c12a5e)
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

2 files changed, 35 insertions, 5 deletions

diff --git a/src/qml/animations/qabstractanimationjob.cpp b/src/qml/animations/qabstractanimationjob.cpp
index 2541253b1b..cdbb66251f 100644
--- a/src/qml/animations/qabstractanimationjob.cpp
+++ b/src/qml/animations/qabstractanimationjob.cpp
@@ -65,6 +65,30 @@ QQmlAnimationTimer::QQmlAnimationTimer() :
 {
 }
 
+void QQmlAnimationTimer::unsetJobTimer(QAbstractAnimationJob *animation)
+{
+    if (!animation)
+        return;
+    if (animation->m_timer == this)
+        animation->m_timer = nullptr;
+
+    if (animation->isGroup()) {
+        QAnimationGroupJob *group = static_cast<QAnimationGroupJob *>(animation);
+        for (auto *child = group->firstChild(); child; child = child->nextSibling())
+            unsetJobTimer(child);
+    }
+}
+
+QQmlAnimationTimer::~QQmlAnimationTimer()
+{
+    for (const auto &animation : qAsConst(animations))
+        unsetJobTimer(animation);
+    for (const auto &animation : qAsConst(animationsToStart))
+        unsetJobTimer(animation);
+    for (const auto &animation : qAsConst(runningPauseAnimations))
+        unsetJobTimer(animation);
+}
+
 QQmlAnimationTimer *QQmlAnimationTimer::instance(bool create)
 {
     QQmlAnimationTimer *inst;
@@ -283,7 +307,8 @@ QAbstractAnimationJob::~QAbstractAnimationJob()
         Q_ASSERT(m_state == Stopped);
         if (oldState == Running) {
             Q_ASSERT(QQmlAnimationTimer::instance(false) == m_timer);
-            m_timer->unregisterAnimation(this);
+            if (m_timer)
+                m_timer->unregisterAnimation(this);
         }
         Q_ASSERT(!m_hasRegisteredTimer);
     }
@@ -308,8 +333,9 @@ void QAbstractAnimationJob::setState(QAbstractAnimationJob::State newState)
     if (m_loopCount == 0)
         return;
 
-    if (!m_timer)
-        m_timer = QQmlAnimationTimer::instance();
+    if (!m_timer) // don't create a timer just to stop the animation
+        m_timer = QQmlAnimationTimer::instance(newState != Stopped);
+    Q_ASSERT(m_timer || newState == Stopped);
 
     State oldState = m_state;
     int oldCurrentTime = m_currentTime;
@@ -337,8 +363,9 @@ void QAbstractAnimationJob::setState(QAbstractAnimationJob::State newState)
     if (oldState == Running) {
         if (newState == Paused && m_hasRegisteredTimer)
             m_timer->ensureTimerUpdate();
-        //the animation, is not running any more
-        m_timer->unregisterAnimation(this);
+        // the animation is not running any more
+        if (m_timer)
+            m_timer->unregisterAnimation(this);
     } else if (newState == Running) {
         m_timer->registerAnimation(this, isTopLevel);
     }
diff --git a/src/qml/animations/qabstractanimationjob_p.h b/src/qml/animations/qabstractanimationjob_p.h
index d6df32df25..8abe11e8f6 100644
--- a/src/qml/animations/qabstractanimationjob_p.h
+++ b/src/qml/animations/qabstractanimationjob_p.h
@@ -207,6 +207,8 @@ private:
     QQmlAnimationTimer();
 
 public:
+    ~QQmlAnimationTimer(); // must be destructible by QThreadStorage
+
     static QQmlAnimationTimer *instance();
     static QQmlAnimationTimer *instance(bool create);
 
@@ -252,6 +254,7 @@ private:
 
     void registerRunningAnimation(QAbstractAnimationJob *animation);
     void unregisterRunningAnimation(QAbstractAnimationJob *animation);
+    void unsetJobTimer(QAbstractAnimationJob *animation);
 
     int closestPauseAnimationTimeToFinish();
 };