From 8f3311276e4ca44acb69c8870ccfc3167682b898 Mon Sep 17 00:00:00 2001 From: Gunnar Sletta Date: Thu, 28 Aug 2014 11:27:02 +0200 Subject: Fix crash with cleanup of animators. We had several separate issues relating to how the jobs were cleaned up. The first was that upon getting setWindow(0), the animator did not reset m_controller to 0, leading to the starts() coming after that to post null jobs to the controller. This would later crash in beforeNodeSync as the starting job was null. The second issue was that during shutdown, QQuickAnimatorProxy would try to delete jobs on the controller which was already deleted. The controller is deleted on the GUI thread regardless of render loop, so this was solved with a QPointer. The third was that we were a bit too aggressive in trying to clean up jobs on the GUI thread, so we introduced a new bool which gets set to true in startJob() so that Proxy::deleteJob() knows who owns the job. Task-number: QTBUG-37833 Change-Id: I1b6221a2c1ce2bfd0758801b950cda00ff6899d0 Reviewed-by: Michael Brasser --- src/quick/util/qquickanimatorcontroller.cpp | 1 + src/quick/util/qquickanimatorjob.cpp | 14 ++++++++++---- src/quick/util/qquickanimatorjob_p.h | 4 +++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/quick/util/qquickanimatorcontroller.cpp b/src/quick/util/qquickanimatorcontroller.cpp index 697c25b211..f8e24bf9fb 100644 --- a/src/quick/util/qquickanimatorcontroller.cpp +++ b/src/quick/util/qquickanimatorcontroller.cpp @@ -262,6 +262,7 @@ void QQuickAnimatorController::requestSync() // These functions are called on the GUI thread. void QQuickAnimatorController::startJob(QQuickAnimatorProxyJob *proxy, QAbstractAnimationJob *job) { + proxy->markJobManagedByController(); m_starting[job] = proxy; requestSync(); } diff --git a/src/quick/util/qquickanimatorjob.cpp b/src/quick/util/qquickanimatorjob.cpp index 9f81f28b7a..d61066babe 100644 --- a/src/quick/util/qquickanimatorjob.cpp +++ b/src/quick/util/qquickanimatorjob.cpp @@ -57,6 +57,7 @@ QQuickAnimatorProxyJob::QQuickAnimatorProxyJob(QAbstractAnimationJob *job, QObje : m_controller(0) , m_job(job) , m_internalState(State_Stopped) + , m_jobManagedByController(false) { m_isRenderThreadProxy = true; m_animation = qobject_cast(item); @@ -101,7 +102,10 @@ void QQuickAnimatorProxyJob::deleteJob() // so delete it through the controller to clean up properly. if (m_controller) m_controller->deleteJob(m_job); - else + + // We explicitly delete the job if the animator controller has never touched + // it. If it has, it will have ownership as well. + else if (!m_jobManagedByController) delete m_job; m_job = 0; } @@ -149,11 +153,13 @@ void QQuickAnimatorProxyJob::controllerWasDeleted() void QQuickAnimatorProxyJob::setWindow(QQuickWindow *window) { if (!window) { - // Stop will trigger syncBackCurrentValues so best to do it before - // we delete m_job. stop(); deleteJob(); - return; + + // Upon leaving a window, we reset the controller. This means that + // animators will only enter the Starting phase and won't be making + // calls to QQuickAnimatorController::startjob(). + m_controller = 0; } else if (!m_controller && m_job) { m_controller = QQuickWindowPrivate::get(window)->animationController; diff --git a/src/quick/util/qquickanimatorjob_p.h b/src/quick/util/qquickanimatorjob_p.h index 03b13bcd30..1ebf12faab 100644 --- a/src/quick/util/qquickanimatorjob_p.h +++ b/src/quick/util/qquickanimatorjob_p.h @@ -77,6 +77,7 @@ public: void startedByController(); void controllerWasDeleted(); + void markJobManagedByController() { m_jobManagedByController = true; } protected: bool event(QEvent *); @@ -95,7 +96,7 @@ private: void setWindow(QQuickWindow *window); static QObject *findAnimationContext(QQuickAbstractAnimation *); - QQuickAnimatorController *m_controller; + QPointer m_controller; QQuickAbstractAnimation *m_animation; QAbstractAnimationJob *m_job; int m_duration; @@ -108,6 +109,7 @@ private: }; InternalState m_internalState; + bool m_jobManagedByController; }; class Q_QUICK_PRIVATE_EXPORT QQuickAnimatorJob : public QAbstractAnimationJob -- cgit v1.2.3