From 1241aa609bb1621e6377920219b6d18a1203e50a Mon Sep 17 00:00:00 2001 From: Leonardo Sobral Cunha Date: Thu, 27 Aug 2009 16:26:38 +0200 Subject: QAbstractAnimation: fixes segfault when deleting animation inside updateCurrentTime That deletion removed the respective animation from the list of running animations being processed, but not from the copy of the list introduced in 48489988521b818bda8d76e60cb272508e91b490, thus we had a dangling pointer. Reviewed-by: thierry --- src/corelib/animation/qabstractanimation.cpp | 27 +++++++---- src/corelib/animation/qabstractanimation_p.h | 1 + .../qpropertyanimation/tst_qpropertyanimation.cpp | 53 ++++++++++++++++++++-- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/corelib/animation/qabstractanimation.cpp b/src/corelib/animation/qabstractanimation.cpp index 617f4db..dfd6779 100644 --- a/src/corelib/animation/qabstractanimation.cpp +++ b/src/corelib/animation/qabstractanimation.cpp @@ -161,7 +161,9 @@ QT_BEGIN_NAMESPACE Q_GLOBAL_STATIC(QThreadStorage, unifiedTimer) -QUnifiedTimer::QUnifiedTimer() : QObject(), lastTick(0), timingInterval(DEFAULT_TIMER_INTERVAL), consistentTiming(false) +QUnifiedTimer::QUnifiedTimer() : + QObject(), lastTick(0), timingInterval(DEFAULT_TIMER_INTERVAL), + currentAnimationIdx(0), consistentTiming(false) { } @@ -200,21 +202,19 @@ void QUnifiedTimer::timerEvent(QTimerEvent *event) } } else if (event->timerId() == animationTimer.timerId()) { const int delta = lastTick - oldLastTick; - //we copy the list so that if it is changed we still get to - //call setCurrentTime on all animations. - const QList currentAnimations = animations; - for (int i = 0; i < currentAnimations.count(); ++i) { - QAbstractAnimation *animation = currentAnimations.at(i); + for (currentAnimationIdx = 0; currentAnimationIdx < animations.count(); ++currentAnimationIdx) { + QAbstractAnimation *animation = animations.at(currentAnimationIdx); int elapsed = QAbstractAnimationPrivate::get(animation)->totalCurrentTime + (animation->direction() == QAbstractAnimation::Forward ? delta : -delta); animation->setCurrentTime(elapsed); } + currentAnimationIdx = 0; } } void QUnifiedTimer::registerAnimation(QAbstractAnimation *animation) { - if (animations.contains(animation) ||animationsToStart.contains(animation)) + if (animations.contains(animation) || animationsToStart.contains(animation)) return; animationsToStart << animation; startStopAnimationTimer.start(0, this); // we delay the check if we should start/stop the global timer @@ -222,8 +222,17 @@ void QUnifiedTimer::registerAnimation(QAbstractAnimation *animation) void QUnifiedTimer::unregisterAnimation(QAbstractAnimation *animation) { - animations.removeAll(animation); - animationsToStart.removeAll(animation); + Q_ASSERT(animations.count(animation) + animationsToStart.count(animation) <= 1); + + int idx = animations.indexOf(animation); + if (idx != -1) { + animations.removeAt(idx); + // this is needed if we unregister an animation while its running + if (idx <= currentAnimationIdx) + --currentAnimationIdx; + } else { + animationsToStart.removeOne(animation); + } startStopAnimationTimer.start(0, this); // we delay the check if we should start/stop the global timer } diff --git a/src/corelib/animation/qabstractanimation_p.h b/src/corelib/animation/qabstractanimation_p.h index 32189f5..bcd7354 100644 --- a/src/corelib/animation/qabstractanimation_p.h +++ b/src/corelib/animation/qabstractanimation_p.h @@ -141,6 +141,7 @@ private: QTime time; int lastTick; int timingInterval; + int currentAnimationIdx; bool consistentTiming; QList animations, animationsToStart; }; diff --git a/tests/auto/qpropertyanimation/tst_qpropertyanimation.cpp b/tests/auto/qpropertyanimation/tst_qpropertyanimation.cpp index 04cfe1a..57fca9d 100644 --- a/tests/auto/qpropertyanimation/tst_qpropertyanimation.cpp +++ b/tests/auto/qpropertyanimation/tst_qpropertyanimation.cpp @@ -66,7 +66,7 @@ protected: class MyObject : public QObject { Q_OBJECT - Q_PROPERTY(qreal x READ x WRITE setX) + Q_PROPERTY(qreal x READ x WRITE setX) public: MyObject() : m_x(0) { } qreal x() const { return m_x; } @@ -115,6 +115,7 @@ private slots: void restart(); void valueChanged(); void twoAnimations(); + void deletedInUpdateCurrentTime(); }; tst_QPropertyAnimation::tst_QPropertyAnimation() @@ -142,7 +143,7 @@ class AnimationObject : public QObject Q_PROPERTY(qreal realValue READ realValue WRITE setRealValue) public: AnimationObject(int startValue = 0) - : v(startValue) + : v(startValue), rv(startValue) { } int value() const { return v; } @@ -402,7 +403,7 @@ void tst_QPropertyAnimation::duration0() animation.setStartValue(42); QVERIFY(animation.currentValue().isValid()); QCOMPARE(animation.currentValue().toInt(), 42); - + QCOMPARE(o.property("ole").toInt(), 42); animation.setDuration(0); QCOMPARE(animation.currentValue().toInt(), 43); //it is at the end @@ -1125,11 +1126,55 @@ void tst_QPropertyAnimation::twoAnimations() QCOMPARE(o1.ole(), 1000); QCOMPARE(o2.ole(), 1000); - } +class MyComposedAnimation : public QPropertyAnimation +{ + Q_OBJECT +public: + MyComposedAnimation(QObject *target, const QByteArray &propertyName, const QByteArray &innerPropertyName) + : QPropertyAnimation(target, propertyName) + { + innerAnim = new QPropertyAnimation(target, innerPropertyName); + this->setEndValue(1000); + innerAnim->setEndValue(1000); + innerAnim->setDuration(duration() + 100); + } + void start() + { + QPropertyAnimation::start(); + innerAnim->start(); + } + void updateState(QAbstractAnimation::State oldState, QAbstractAnimation::State newState) + { + QPropertyAnimation::updateState(oldState, newState); + if (newState == QAbstractAnimation::Stopped) + delete innerAnim; + } + +public: + QPropertyAnimation *innerAnim; +}; + +void tst_QPropertyAnimation::deletedInUpdateCurrentTime() +{ + // this test case reproduces an animation being deleted in the updateCurrentTime of + // another animation(was causing segfault). + // the deleted animation must have been started after the animation that is deleting. + AnimationObject o; + o.setValue(0); + o.setRealValue(0.0); + + MyComposedAnimation composedAnimation(&o, "value", "realValue"); + composedAnimation.start(); + QCOMPARE(composedAnimation.state(), QAbstractAnimation::Running); + QTest::qWait(composedAnimation.duration() + 50); + + QCOMPARE(composedAnimation.state(), QAbstractAnimation::Stopped); + QCOMPARE(o.value(), 1000); +} QTEST_MAIN(tst_QPropertyAnimation) #include "tst_qpropertyanimation.moc" -- cgit v0.12