From 36a7259405aae9ce1ea474e28029e42db7d589fc Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Wed, 17 Nov 2021 11:05:35 +0100 Subject: 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 Reviewed-by: Qt CI Bot (cherry picked from commit 9c732286ed72afe8cc2d2062e6cedbe5f6a06447) Reviewed-by: Qt Cherry-pick Bot --- src/qml/animations/qabstractanimationjob.cpp | 6 ++-- .../data/cleanupWhenRenderThreadStops.qml | 32 ++++++++++++++++++++++ .../qquickanimations/tst_qquickanimations.cpp | 14 ++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/auto/quick/qquickanimations/data/cleanupWhenRenderThreadStops.qml 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(),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" -- cgit v1.2.3