1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QStateMachine: handle parallel child mode for state machines

Browse Source 
Setting the childMode property to ParallelStates will result in an
invalid state machine. This is never checked (worse, we explicitly
allow it and have a constructor to set it), but it results in
findLCCA failing, which then results in a failing assert or crash.

This fix in this patch is to handle this case separately. The proper
fix would be to remove completely the ability to set the childMode
on a QStateMachine, but that will have to wait until Qt6.

Fixes: QTBUG-49975
Change-Id: I43692309c4d438ee1a9bc55fa4f65f8bce8e0a59
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Erik Verbruggen 2019-04-16 13:30:31 +02:00 committed by Ulf Hermann
parent d7afd8bb38
commit cfdbfcebbd
4 changed files with 53 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/corelib/statemachine/qstatemachine.cpp
View File

@ -140,6 +140,10 @@ QT_BEGIN_NAMESPACE
no error state applies to the erroneous state, the machine will stop
executing and an error message will be printed to the console.
\note Important: setting the \l{ChildMode} of a state machine to parallel (\l{ParallelStates})
results in an invalid state machine. It can only be set to (or kept as)
\l{ExclusiveStates}.
\sa QAbstractState, QAbstractTransition, QState, {The State Machine Framework}
*/
@ -519,7 +523,7 @@ bool QStateMachinePrivate::stateExitLessThan(QAbstractState *s1, QAbstractState
}
}
QState *QStateMachinePrivate::findLCA(const QList<QAbstractState*> &states, bool onlyCompound) const
QState *QStateMachinePrivate::findLCA(const QList<QAbstractState*> &states, bool onlyCompound)
{
if (states.isEmpty())
return 0;
@ -538,10 +542,16 @@ QState *QStateMachinePrivate::findLCA(const QList<QAbstractState*> &states, bool
if (ok)
return anc;
}
return 0;
// Oops, this should never happen! The state machine itself is a common ancestor of all states,
// no matter what. But, for the onlyCompound case: we probably have a state machine whose
// childMode is set to parallel, which is illegal. However, we're stuck with it (and with
// exposing this invalid/dangerous API to users), so recover in the least horrible way.
setError(QStateMachine::StateMachineChildModeSetToParallelError, q_func());
return q_func(); // make the statemachine the LCA/LCCA (which it should have been anyway)
}
QState *QStateMachinePrivate::findLCCA(const QList<QAbstractState*> &states) const
QState *QStateMachinePrivate::findLCCA(const QList<QAbstractState*> &states)
{
return findLCA(states, true);
}
@ -903,7 +913,7 @@ function getTransitionDomain(t)
*/
QAbstractState *QStateMachinePrivate::getTransitionDomain(QAbstractTransition *t,
const QList<QAbstractState *> &effectiveTargetStates,
CalculationCache *cache) const
CalculationCache *cache)
{
Q_ASSERT(cache);
@ -1484,6 +1494,14 @@ void QStateMachinePrivate::setError(QStateMachine::Error errorCode, QAbstractSta
errorString = QStateMachine::tr("No common ancestor for targets and source of transition from state '%1'")
.arg(currentContext->objectName());
break;
case QStateMachine::StateMachineChildModeSetToParallelError:
Q_ASSERT(currentContext != nullptr);
errorString = QStateMachine::tr("Child mode of state machine '%1' is not 'ExclusiveStates'!")
.arg(currentContext->objectName());
break;
default:
errorString = QStateMachine::tr("Unknown error");
};
@ -2445,9 +2463,13 @@ QStateMachine::QStateMachine(QObject *parent)
/*!
\since 5.0
\deprecated
Constructs a new state machine with the given \a childMode
and \a parent.
\warning Do not set the \a childMode to anything else than \l{ExclusiveStates}, otherwise the
state machine is invalid, and might work incorrectly!
*/
QStateMachine::QStateMachine(QState::ChildMode childMode, QObject *parent)
: QState(*new QStateMachinePrivate, /*parentState=*/0)
@ -2455,6 +2477,18 @@ QStateMachine::QStateMachine(QState::ChildMode childMode, QObject *parent)
Q_D(QStateMachine);
d->childMode = childMode;
setParent(parent); // See comment in constructor above
if (childMode != ExclusiveStates) {
//### FIXME for Qt6: remove this constructor completely, and hide the childMode property.
// Yes, the StateMachine itself is conceptually a state, but it should only expose a limited
// number of properties. The execution algorithm (in the URL below) treats a state machine
// as a state, but from an API point of view, it's questionable if the QStateMachine should
// inherit from QState.
//
// See function findLCCA in https://www.w3.org/TR/2014/WD-scxml-20140529/#AlgorithmforSCXMLInterpretation
// to see where setting childMode to parallel will break down.
qWarning() << "Invalid childMode for QStateMachine" << this;
}
}
/*!

3
src/corelib/statemachine/qstatemachine.h
View File

@ -106,7 +106,8 @@ public:
NoError,
NoInitialStateError,
NoDefaultStateInHistoryStateError,
NoCommonAncestorForTransitionError
NoCommonAncestorForTransitionError,
StateMachineChildModeSetToParallelError
};
explicit QStateMachine(QObject *parent = nullptr);

6
src/corelib/statemachine/qstatemachine_p.h
View File

@ -110,8 +110,8 @@ public:
static QStateMachinePrivate *get(QStateMachine *q)
{ return q ? q->d_func() : nullptr; }
QState *findLCA(const QList<QAbstractState*> &states, bool onlyCompound = false) const;
QState *findLCCA(const QList<QAbstractState*> &states) const;
QState *findLCA(const QList<QAbstractState*> &states, bool onlyCompound = false);
QState *findLCCA(const QList<QAbstractState*> &states);
static bool transitionStateEntryLessThan(QAbstractTransition *t1, QAbstractTransition *t2);
static bool stateEntryLessThan(QAbstractState *s1, QAbstractState *s2);
@ -160,7 +160,7 @@ public:
QSet<QAbstractState*> &statesForDefaultEntry, CalculationCache *cache);
QAbstractState *getTransitionDomain(QAbstractTransition *t,
const QList<QAbstractState *> &effectiveTargetStates,
CalculationCache *cache) const;
CalculationCache *cache);
void addDescendantStatesToEnter(QAbstractState *state,
QSet<QAbstractState*> &statesToEnter,
QSet<QAbstractState*> &statesForDefaultEntry);

14
tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp
View File

@ -336,7 +336,9 @@ void tst_QStateMachine::transitionToRootState()
TEST_ACTIVE_CHANGED(initialState, 1);
machine.postEvent(new QEvent(QEvent::User));
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: No common ancestor for targets and source of transition from state 'initial'");
QTest::ignoreMessage(QtWarningMsg,
"Unrecoverable error detected in running state machine: "
"Child mode of state machine 'machine' is not 'ExclusiveStates'!");
QCoreApplication::processEvents();
QVERIFY(machine.configuration().isEmpty());
QVERIFY(!machine.isRunning());
@ -1061,7 +1063,8 @@ void tst_QStateMachine::transitionToStateNotInGraph()
initialState->addTransition(&independentState);
machine.start();
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: No common ancestor for targets and source of transition from state 'initialState'");
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: "
"Child mode of state machine '' is not 'ExclusiveStates'!");
QCoreApplication::processEvents();
QCOMPARE(machine.isRunning(), false);
@ -2099,6 +2102,8 @@ void tst_QStateMachine::parallelRootState()
QSignalSpy finishedSpy(&machine, &QStateMachine::finished);
QVERIFY(finishedSpy.isValid());
machine.start();
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: "
"Child mode of state machine '' is not 'ExclusiveStates'!");
QTRY_COMPARE(startedSpy.count(), 1);
QCOMPARE(machine.configuration().size(), 4);
QVERIFY(machine.configuration().contains(s1));
@ -3310,14 +3315,15 @@ void tst_QStateMachine::targetStateWithNoParent()
QVERIFY(runningSpy.isValid());
machine.start();
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: No common ancestor for targets and source of transition from state 's1'");
QTest::ignoreMessage(QtWarningMsg, "Unrecoverable error detected in running state machine: "
"Child mode of state machine '' is not 'ExclusiveStates'!");
TEST_ACTIVE_CHANGED(s1, 2);
QTRY_COMPARE(startedSpy.count(), 1);
QCOMPARE(machine.isRunning(), false);
QCOMPARE(stoppedSpy.count(), 1);
QCOMPARE(finishedSpy.count(), 0);
TEST_RUNNING_CHANGED_STARTED_STOPPED;
QCOMPARE(machine.error(), QStateMachine::NoCommonAncestorForTransitionError);
QCOMPARE(machine.error(), QStateMachine::StateMachineChildModeSetToParallelError);
}
void tst_QStateMachine::targetStateDeleted()