statemachine: Fix signal transition handling in multi-threaded setup

By default, QStateMachine lazily registers signal transitions (i.e.,
connects to the signal) when the transition's source state is
entered. The connections are established in Qt::AutoConnection mode,
which means that if the sender object lives in a different thread,
the signal processing will be queued.

But if a sender object's signal is used in an out-going transition
of the target state of the queued transition, it's possible that a
second signal emission on the sender object's thread will be
"missed" by the state machine; before the machine gets around to
processing the first queued emission (and registering the
transitions of the new state), a sender object on the other thread
could have emitted a new signal.

The solution employed here is to eagerly register any signal
transition whose sender object is on a different thread; that is,
register it regardless of whether the transition's source state is
active.

Conversely, when a machine's transitions are unregistered (i.e.,
because the machine finished), signal transitions with sender
objects on other threads should be left as-is, in case the machine
will be run again.

This doesn't solve the case where the sender object is moved to a
different thread _after_ the transition has been initialized.
Theoretically, we could catch that by installing an event filter
on every sender object and handle the ThreadChange events, but
that would be very expensive, and likely useless in most cases.
So let's just say that that case isn't supported for now.

Task-number: QTBUG-19789
Change-Id: Ibc87bfbf2ed83217ac61ae9401fe4f179ef26c24
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>

src/corelib/statemachine/qstatemachine.cpp

@@ -1703,8 +1703,10 @@ void QStateMachinePrivate::unregisterTransition(QAbstractTransition *transition)
 void QStateMachinePrivate::maybeRegisterSignalTransition(QSignalTransition *transition)
 {
     Q_Q(QStateMachine);
-    if ((state == Running) && configuration.contains(transition->sourceState()))
+    if (((state == Running) && configuration.contains(transition->sourceState()))
+            || (transition->senderObject() && (transition->senderObject()->thread() != q->thread()))) {
         registerSignalTransition(transition);
+    }
 }
 
 void QStateMachinePrivate::registerSignalTransition(QSignalTransition *transition)
@@ -1766,11 +1768,15 @@ void QStateMachinePrivate::registerSignalTransition(QSignalTransition *transitio
 
 void QStateMachinePrivate::unregisterSignalTransition(QSignalTransition *transition)
 {
+    Q_Q(QStateMachine);
     int signalIndex = QSignalTransitionPrivate::get(transition)->signalIndex;
     if (signalIndex == -1)
         return; // not registered
-    QSignalTransitionPrivate::get(transition)->signalIndex = -1;
     const QObject *sender = QSignalTransitionPrivate::get(transition)->sender;
+    // Don't unregister if the sender is on another thread
+    if (!sender || (sender->thread() != q->thread()))
+        return;
+    QSignalTransitionPrivate::get(transition)->signalIndex = -1;
     QVector<int> &connectedSignalIndexes = connections[sender];
     Q_ASSERT(connectedSignalIndexes.size() > signalIndex);
     Q_ASSERT(connectedSignalIndexes.at(signalIndex) != 0);

tests/auto/corelib/statemachine/qstatemachine/tst_qstatemachine.cpp

@@ -208,6 +208,7 @@ private slots:
     void signalTransitionNormalizeSignature();
     void createSignalTransitionWhenRunning();
     void createEventTransitionWhenRunning();
+    void signalTransitionSenderInDifferentThread();
 };
 
 class TestState : public QState
@@ -4827,5 +4828,61 @@ void tst_QStateMachine::createEventTransitionWhenRunning()
     QTRY_VERIFY(machine.configuration().contains(s4));
 }
 
+class SignalEmitterThread : public QThread
+{
+    Q_OBJECT
+public:
+    SignalEmitterThread(QObject *parent = 0)
+        : QThread(parent)
+    {
+        moveToThread(this);
+    }
+
+Q_SIGNALS:
+    void signal1();
+    void signal2();
+
+public Q_SLOTS:
+    void emitSignals()
+    {
+        emit signal1();
+        emit signal2();
+    }
+};
+
+// QTBUG-19789
+void tst_QStateMachine::signalTransitionSenderInDifferentThread()
+{
+    QStateMachine machine;
+    QState *s1 = new QState(&machine);
+    machine.setInitialState(s1);
+
+    SignalEmitterThread thread;
+    QState *s2 = new QState(&machine);
+    s1->addTransition(&thread, SIGNAL(signal1()), s2);
+
+    QFinalState *s3 = new QFinalState(&machine);
+    s2->addTransition(&thread, SIGNAL(signal2()), s3);
+
+    thread.start();
+    QTRY_VERIFY(thread.isRunning());
+
+    machine.start();
+    QTRY_VERIFY(machine.configuration().contains(s1));
+
+    QMetaObject::invokeMethod(&thread, "emitSignals");
+    // thread emits both signal1() and signal2(), so we should end in s3
+    QTRY_VERIFY(machine.configuration().contains(s3));
+
+    // Run the machine again; transitions should still be registered
+    machine.start();
+    QTRY_VERIFY(machine.configuration().contains(s1));
+    QMetaObject::invokeMethod(&thread, "emitSignals");
+    QTRY_VERIFY(machine.configuration().contains(s3));
+
+    thread.quit();
+    QTRY_VERIFY(thread.wait());
+}
+
 QTEST_MAIN(tst_QStateMachine)
 #include "tst_qstatemachine.moc"