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.

Pick-to: 6.2
Change-Id: Ice0c83818dd712c9e3be1261b79631f639de61a0
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

1 files changed, 35 insertions, 5 deletions

diff --git a/src/qml/animations/qabstractanimationjob.cpp b/src/qml/animations/qabstractanimationjob.cpp
index e975b3e29e..76e3f2b7e6 100644
--- a/src/qml/animations/qabstractanimationjob.cpp
+++ b/src/qml/animations/qabstractanimationjob.cpp
@@ -44,6 +44,7 @@
 #include "private/qanimationjobutil_p.h"
 #include "private/qqmlengine_p.h"
 #include "private/qqmlglobal_p.h"
+#include "private/qdoubleendedlist_p.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -65,6 +66,32 @@ 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);
+        if (const auto children = group->children()) {
+            for (auto *child : *children)
+                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;
@@ -281,7 +308,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);
     }
@@ -306,8 +334,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;
@@ -335,8 +364,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);
     }