From ba87568655dad3e830e692109b5e571ae78b71a0 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Mon, 2 Jul 2012 20:45:49 +0200 Subject: [PATCH] statemachine: Don't crash if property assignment target is deleted Do like QPropertyAnimation and store the QObject in a QPointer. Purge the assignments list upon state entry and property restore. Change-Id: I54a56885a2905178ab6aa5cf292b3d25c86b7a97 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/corelib/statemachine/qstate_p.h | 6 ++- src/corelib/statemachine/qstatemachine.cpp | 40 ++++++++++---- src/corelib/statemachine/qstatemachine_p.h | 3 +- .../qstatemachine/tst_qstatemachine.cpp | 52 +++++++++++++++++++ 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/corelib/statemachine/qstate_p.h b/src/corelib/statemachine/qstate_p.h index 30be47cf51..9558bd88a5 100644 --- a/src/corelib/statemachine/qstate_p.h +++ b/src/corelib/statemachine/qstate_p.h @@ -57,6 +57,7 @@ #include #include +#include #include QT_BEGIN_NAMESPACE @@ -69,7 +70,10 @@ struct QPropertyAssignment const QVariant &v, bool es = true) : object(o), propertyName(n), value(v), explicitlySet(es) {} - QObject *object; + + bool objectDeleted() const { return !object; } + + QPointer object; QByteArray propertyName; QVariant value; bool explicitlySet; diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp index 3d0b1e92fd..24da4e6937 100644 --- a/src/corelib/statemachine/qstatemachine.cpp +++ b/src/corelib/statemachine/qstatemachine.cpp @@ -176,6 +176,10 @@ QT_BEGIN_NAMESPACE // #define QSTATEMACHINE_DEBUG +template +static uint qHash(const QPointer &p) +{ return qHash(p.data()); } + QStateMachinePrivate::QStateMachinePrivate() { isMachine = true; @@ -666,14 +670,20 @@ void QStateMachinePrivate::applyProperties(const QList &tr if (!s) continue; - QList assignments = QStatePrivate::get(s)->propertyAssignments; - for (int j = 0; j < assignments.size(); ++j) { - const QPropertyAssignment &assn = assignments.at(j); - if (globalRestorePolicy == QStateMachine::RestoreProperties) { - registerRestorable(assn.object, assn.propertyName); + { + QList &assignments = QStatePrivate::get(s)->propertyAssignments; + for (int j = 0; j < assignments.size(); ++j) { + const QPropertyAssignment &assn = assignments.at(j); + if (assn.objectDeleted()) { + assignments.removeAt(j--); + } else { + if (globalRestorePolicy == QStateMachine::RestoreProperties) { + registerRestorable(assn.object, assn.propertyName); + } + pendingRestorables.remove(RestorableId(assn.object, assn.propertyName)); + propertyAssignmentsForState[s].append(assn); + } } - pendingRestorables.remove(RestorableId(assn.object, assn.propertyName)); - propertyAssignmentsForState[s].append(assn); } // Remove pending restorables for all parent states to avoid restoring properties @@ -681,12 +691,16 @@ void QStateMachinePrivate::applyProperties(const QList &tr // assign a property which is assigned by the parent, it inherits the parent's assignment. QState *parentState = s; while ((parentState = parentState->parentState()) != 0) { - assignments = QStatePrivate::get(parentState)->propertyAssignments; + QList &assignments = QStatePrivate::get(parentState)->propertyAssignments; for (int j=0; j 0) - propertyAssignmentsForState[s].append(assn); + if (assn.objectDeleted()) { + assignments.removeAt(j--); + } else { + int c = pendingRestorables.remove(RestorableId(assn.object, assn.propertyName)); + if (c > 0) + propertyAssignmentsForState[s].append(assn); + } } } } @@ -967,6 +981,10 @@ QList QStateMachinePrivate::restorablesToPropertyList(const QHash::const_iterator it; for (it = restorables.constBegin(); it != restorables.constEnd(); ++it) { // qDebug() << "restorable:" << it.key().first << it.key().second << it.value(); + if (!it.key().first) { + // Property object was deleted + continue; + } result.append(QPropertyAssignment(it.key().first, it.key().second, it.value(), /*explicitlySet=*/false)); } return result; diff --git a/src/corelib/statemachine/qstatemachine_p.h b/src/corelib/statemachine/qstatemachine_p.h index ae5660719f..6159107dc0 100644 --- a/src/corelib/statemachine/qstatemachine_p.h +++ b/src/corelib/statemachine/qstatemachine_p.h @@ -60,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -183,7 +184,7 @@ public: void cancelAllDelayedEvents(); #ifndef QT_NO_PROPERTIES - typedef QPair RestorableId; + typedef QPair, QByteArray> RestorableId; QHash registeredRestorables; void registerRestorable(QObject *object, const QByteArray &propertyName); void unregisterRestorable(QObject *object, const QByteArray &propertyName); diff --git a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp index 862ff96760..6fde415f2a 100644 --- a/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp +++ b/tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp @@ -187,6 +187,8 @@ private slots: void testIncrementReceivers(); void initialStateIsEnteredBeforeStartedEmitted(); + void deletePropertyAssignmentObjectBeforeEntry(); + void deletePropertyAssignmentObjectBeforeRestore(); }; class TestState : public QState @@ -4021,5 +4023,55 @@ void tst_QStateMachine::initialStateIsEnteredBeforeStartedEmitted() QTRY_COMPARE(finishedSpy.count(), 1); } +void tst_QStateMachine::deletePropertyAssignmentObjectBeforeEntry() +{ + QStateMachine machine; + QState *s1 = new QState(&machine); + machine.setInitialState(s1); + + QObject *o1 = new QObject; + s1->assignProperty(o1, "objectName", "foo"); + delete o1; + QObject *o2 = new QObject; + s1->assignProperty(o2, "objectName", "bar"); + + machine.start(); + // Shouldn't crash + QTRY_VERIFY(machine.configuration().contains(s1)); + + QCOMPARE(o2->objectName(), QString::fromLatin1("bar")); + delete o2; +} + +void tst_QStateMachine::deletePropertyAssignmentObjectBeforeRestore() +{ + QStateMachine machine; + machine.setGlobalRestorePolicy(QStateMachine::RestoreProperties); + QState *s1 = new QState(&machine); + machine.setInitialState(s1); + QState *s2 = new QState(&machine); + s1->addTransition(new EventTransition(QEvent::User, s2)); + + QObject *o1 = new QObject; + s1->assignProperty(o1, "objectName", "foo"); + QObject *o2 = new QObject; + s1->assignProperty(o2, "objectName", "bar"); + + QVERIFY(o1->objectName().isEmpty()); + QVERIFY(o2->objectName().isEmpty()); + machine.start(); + QTRY_VERIFY(machine.configuration().contains(s1)); + QCOMPARE(o1->objectName(), QString::fromLatin1("foo")); + QCOMPARE(o2->objectName(), QString::fromLatin1("bar")); + + delete o1; + machine.postEvent(new QEvent(QEvent::User)); + // Shouldn't crash + QTRY_VERIFY(machine.configuration().contains(s2)); + + QVERIFY(o2->objectName().isEmpty()); + delete o2; +} + QTEST_MAIN(tst_QStateMachine) #include "tst_qstatemachine.moc"