summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorUlf Hermann <ulf.hermann@qt.io>2021-02-05 13:18:25 +0100
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2021-02-05 16:21:11 +0000
commit62bf7be90ba5d7868cc18c8a1e81209c91f449c5 (patch)
tree4cf6cd79e7a46408b456d567134451b905199711
parent41f39169108b1e43ccfca32dc73ceaf16d2a41c8 (diff)
QSequentialAnimationGroupJob: Protect against self-deletion
setCurrentAnimation() can indirectly delete the animation group job itself by invoking the animation controller. Use the RETURN_IF_DELETED mechanism to avoid the resulting dangling pointers. Task-number: QTBUG-90401 Change-Id: Ibd0ad21e8d3af4760604c3ff37dc46101d5f49ad Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io> (cherry picked from commit 680f28b08f65ad38c8d5498b5738231b2a2779a3) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/qml/animations/qanimationjobutil_p.h4
-rw-r--r--src/qml/animations/qsequentialanimationgroupjob.cpp14
-rw-r--r--tests/auto/qml/animation/qsequentialanimationgroupjob/tst_qsequentialanimationgroupjob.cpp42
3 files changed, 50 insertions, 10 deletions
diff --git a/src/qml/animations/qanimationjobutil_p.h b/src/qml/animations/qanimationjobutil_p.h
index 83cf3b246f..2b7bda3123 100644
--- a/src/qml/animations/qanimationjobutil_p.h
+++ b/src/qml/animations/qanimationjobutil_p.h
@@ -70,7 +70,7 @@ struct SelfDeletable {
// \param func statements or functions that to be executed under test.
// \param action post process if p was deleted under test.
#define ACTION_IF_DELETED(p, func, action) \
-{ \
+do { \
static_assert(std::is_same<decltype((p)->m_selfDeletable), SelfDeletable>::value, "m_selfDeletable must be SelfDeletable");\
bool *prevWasDeleted = (p)->m_selfDeletable.m_wasDeleted; \
bool wasDeleted = false; \
@@ -82,7 +82,7 @@ struct SelfDeletable {
{action;} \
} \
(p)->m_selfDeletable.m_wasDeleted = prevWasDeleted; \
-}
+} while (false)
#define RETURN_IF_DELETED(func) \
ACTION_IF_DELETED(this, func, return)
diff --git a/src/qml/animations/qsequentialanimationgroupjob.cpp b/src/qml/animations/qsequentialanimationgroupjob.cpp
index dc57444b32..1d19bbf79d 100644
--- a/src/qml/animations/qsequentialanimationgroupjob.cpp
+++ b/src/qml/animations/qsequentialanimationgroupjob.cpp
@@ -338,7 +338,7 @@ void QSequentialAnimationGroupJob::uncontrolledAnimationFinished(QAbstractAnimat
if (m_direction == Forward) {
// set the current animation to be the next one
if (m_currentAnimation->nextSibling())
- setCurrentAnimation(m_currentAnimation->nextSibling());
+ RETURN_IF_DELETED(setCurrentAnimation(m_currentAnimation->nextSibling()));
for (QAbstractAnimationJob *a = animation->nextSibling(); a; a = a->nextSibling()) {
int dur = a->duration();
@@ -353,7 +353,7 @@ void QSequentialAnimationGroupJob::uncontrolledAnimationFinished(QAbstractAnimat
} else {
// set the current animation to be the previous one
if (m_currentAnimation->previousSibling())
- setCurrentAnimation(m_currentAnimation->previousSibling());
+ RETURN_IF_DELETED(setCurrentAnimation(m_currentAnimation->previousSibling()));
for (QAbstractAnimationJob *a = animation->previousSibling(); a; a = a->previousSibling()) {
int dur = a->duration();
@@ -374,12 +374,12 @@ void QSequentialAnimationGroupJob::uncontrolledAnimationFinished(QAbstractAnimat
void QSequentialAnimationGroupJob::animationInserted(QAbstractAnimationJob *anim)
{
if (m_currentAnimation == nullptr)
- setCurrentAnimation(firstChild()); // initialize the current animation
+ RETURN_IF_DELETED(setCurrentAnimation(firstChild())); // initialize the current animation
if (m_currentAnimation == anim->nextSibling()
&& m_currentAnimation->currentTime() == 0 && m_currentAnimation->currentLoop() == 0) {
//in this case we simply insert the animation before the current one has actually started
- setCurrentAnimation(anim);
+ RETURN_IF_DELETED(setCurrentAnimation(anim));
}
// TODO
@@ -398,11 +398,11 @@ void QSequentialAnimationGroupJob::animationRemoved(QAbstractAnimationJob *anim,
bool removingCurrent = anim == m_currentAnimation;
if (removingCurrent) {
if (next)
- setCurrentAnimation(next); //let's try to take the next one
+ RETURN_IF_DELETED(setCurrentAnimation(next)); //let's try to take the next one
else if (prev)
- setCurrentAnimation(prev);
+ RETURN_IF_DELETED(setCurrentAnimation(prev));
else// case all animations were removed
- setCurrentAnimation(nullptr);
+ RETURN_IF_DELETED(setCurrentAnimation(nullptr));
}
// duration of the previous animations up to the current animation
diff --git a/tests/auto/qml/animation/qsequentialanimationgroupjob/tst_qsequentialanimationgroupjob.cpp b/tests/auto/qml/animation/qsequentialanimationgroupjob/tst_qsequentialanimationgroupjob.cpp
index 4718eb33b4..4bbfaffc1a 100644
--- a/tests/auto/qml/animation/qsequentialanimationgroupjob/tst_qsequentialanimationgroupjob.cpp
+++ b/tests/auto/qml/animation/qsequentialanimationgroupjob/tst_qsequentialanimationgroupjob.cpp
@@ -68,6 +68,7 @@ private slots:
void insertAnimation();
void clear();
void pauseResume();
+ void deleteFromListener();
};
void tst_QSequentialAnimationGroupJob::initTestCase()
@@ -126,15 +127,23 @@ protected:
class StateChangeListener: public QAnimationJobChangeListener
{
public:
- virtual void animationStateChanged(QAbstractAnimationJob *, QAbstractAnimationJob::State newState, QAbstractAnimationJob::State)
+ virtual void animationStateChanged(
+ QAbstractAnimationJob *job, QAbstractAnimationJob::State newState,
+ QAbstractAnimationJob::State)
{
states << newState;
+ if (beEvil) {
+ delete job->group();
+ groupDeleted = true;
+ }
}
void clear() { states.clear(); }
int count() const { return states.count(); }
QList<QAbstractAnimationJob::State> states;
+ bool beEvil = false;
+ bool groupDeleted = false;
};
class FinishedListener: public QAnimationJobChangeListener
@@ -1650,6 +1659,37 @@ void tst_QSequentialAnimationGroupJob::uncontrolledWithLoops()
QTRY_COMPARE(group.state(), QAbstractAnimationJob::Stopped);
}
+void tst_QSequentialAnimationGroupJob::deleteFromListener()
+{
+ QSequentialAnimationGroupJob *group = new QSequentialAnimationGroupJob;
+
+ UncontrolledAnimation *uncontrolled = new UncontrolledAnimation();
+ TestAnimation *shortLoop = new TestAnimation(100);
+ UncontrolledAnimation *more = new UncontrolledAnimation();
+
+ shortLoop->setLoopCount(-1);
+
+ group->appendAnimation(uncontrolled);
+ group->appendAnimation(shortLoop);
+ group->appendAnimation(more);
+
+ StateChangeListener listener;
+ listener.beEvil = true;
+ shortLoop->addAnimationChangeListener(&listener, QAbstractAnimationJob::StateChange);
+ group->setLoopCount(2);
+
+ group->start();
+
+ QCOMPARE(group->currentLoop(), 0);
+ QCOMPARE(group->state(), QAbstractAnimationJob::Running);
+ QTRY_COMPARE(uncontrolled->state(), QAbstractAnimationJob::Running);
+
+ QVERIFY(!listener.groupDeleted);
+ uncontrolled->stop();
+
+ QTRY_VERIFY(listener.groupDeleted);
+ // It's dead, Jim.
+}
QTEST_MAIN(tst_QSequentialAnimationGroupJob)
#include "tst_qsequentialanimationgroupjob.moc"