diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-11-17 11:05:35 +0100 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-11-24 05:38:11 +0000 |
commit | 9c732286ed72afe8cc2d2062e6cedbe5f6a06447 (patch) | |
tree | 9e5befffc858793565582a47afb26b5b6273cf6b | |
parent | a98747ab6c153e1d361ec64ea616764a71465fa4 (diff) |
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
Pick-to: 6.2 6.2.2 5.15
Change-Id: Ief991900c50aefd480d9c79e83324968102ca29c
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_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" |