Make QStateMachine event posting functions thread-safe

By popular demand on the Qt Labs blog.
This makes it possible to readily use QStateMachine with e.g. worker
threads that post events to the machine.

Reviewed-by: Eskil Abrahamsen Blomfeldt

3 files changed, 131 insertions, 17 deletions

diff --git a/src/corelib/statemachine/qstatemachine.cpp b/src/corelib/statemachine/qstatemachine.cpp
index 154445f..45b0286 100644
--- a/src/corelib/statemachine/qstatemachine.cpp
+++ b/src/corelib/statemachine/qstatemachine.cpp
@@ -1216,8 +1216,7 @@ void QStateMachinePrivate::_q_process()
             delete e;
             e = 0;
         }
-        if (enabledTransitions.isEmpty() && !internalEventQueue.isEmpty()) {
-            e = internalEventQueue.takeFirst();
+        if (enabledTransitions.isEmpty() && ((e = dequeueInternalEvent()) != 0)) {
 #ifdef QSTATEMACHINE_DEBUG
             qDebug() << q << ": dequeued internal event" << e << "of type" << e->type();
 #endif
@@ -1228,13 +1227,7 @@ void QStateMachinePrivate::_q_process()
             }
         }
         if (enabledTransitions.isEmpty()) {
-            if (externalEventQueue.isEmpty()) {
-                if (internalEventQueue.isEmpty()) {
-                    processing = false;
-                    stopProcessingReason = EventQueueEmpty;
-                }
-            } else {
-                e = externalEventQueue.takeFirst();
+            if ((e = dequeueExternalEvent()) != 0) {
 #ifdef QSTATEMACHINE_DEBUG
                 qDebug() << q << ": dequeued external event" << e << "of type" << e->type();
 #endif
@@ -1243,6 +1236,11 @@ void QStateMachinePrivate::_q_process()
                     delete e;
                     e = 0;
                 }
+            } else {
+                if (isInternalEventQueueEmpty()) {
+                    processing = false;
+                    stopProcessingReason = EventQueueEmpty;
+                }
             }
         }
         if (!enabledTransitions.isEmpty()) {
@@ -1278,17 +1276,60 @@ void QStateMachinePrivate::_q_process()
     }
 }
 
+void QStateMachinePrivate::postInternalEvent(QEvent *e)
+{
+    QMutexLocker locker(&internalEventMutex);
+    internalEventQueue.append(e);
+}
+
+void QStateMachinePrivate::postExternalEvent(QEvent *e)
+{
+    QMutexLocker locker(&externalEventMutex);
+    externalEventQueue.append(e);
+}
+
+QEvent *QStateMachinePrivate::dequeueInternalEvent()
+{
+    QMutexLocker locker(&internalEventMutex);
+    if (internalEventQueue.isEmpty())
+        return 0;
+    return internalEventQueue.takeFirst();
+}
+
+QEvent *QStateMachinePrivate::dequeueExternalEvent()
+{
+    QMutexLocker locker(&externalEventMutex);
+    if (externalEventQueue.isEmpty())
+        return 0;
+    return externalEventQueue.takeFirst();
+}
+
+bool QStateMachinePrivate::isInternalEventQueueEmpty()
+{
+    QMutexLocker locker(&internalEventMutex);
+    return internalEventQueue.isEmpty();
+}
+
+bool QStateMachinePrivate::isExternalEventQueueEmpty()
+{
+    QMutexLocker locker(&externalEventMutex);
+    return externalEventQueue.isEmpty();
+}
+
 void QStateMachinePrivate::processEvents(EventProcessingMode processingMode)
 {
+    Q_Q(QStateMachine);
     if ((state != Running) || processing || processingScheduled)
         return;
     switch (processingMode) {
     case DirectProcessing:
-        _q_process();
-        break;
+        if (QThread::currentThread() == q->thread()) {
+            _q_process();
+            break;
+        } // fallthrough -- processing must be done in the machine thread
     case QueuedProcessing:
         processingScheduled = true;
-        QMetaObject::invokeMethod(q_func(), "_q_process", Qt::QueuedConnection);
+        QMetaObject::invokeMethod(q, "_q_process", Qt::QueuedConnection);
         break;
     }
 }
@@ -1296,6 +1337,7 @@ void QStateMachinePrivate::processEvents(EventProcessingMode processingMode)
 void QStateMachinePrivate::cancelAllDelayedEvents()
 {
     Q_Q(QStateMachine);
+    QMutexLocker locker(&delayedEventsMutex);
     QHash<int, QEvent*>::const_iterator it;
     for (it = delayedEvents.constBegin(); it != delayedEvents.constEnd(); ++it) {
         int id = it.key();
@@ -1547,7 +1589,7 @@ void QStateMachinePrivate::handleFilteredEvent(QObject *watched, QEvent *event)
 {
     Q_ASSERT(qobjectEvents.contains(watched));
     if (qobjectEvents[watched].contains(event->type())) {
-        internalEventQueue.append(new QStateMachine::WrappedEvent(watched, handler->cloneEvent(event)));
+        postInternalEvent(new QStateMachine::WrappedEvent(watched, handler->cloneEvent(event)));
         processEvents(DirectProcessing);
     }
 }
@@ -1571,7 +1613,7 @@ void QStateMachinePrivate::handleTransitionSignal(QObject *sender, int signalInd
     qDebug() << q_func() << ": sending signal event ( sender =" << sender
              << ", signal =" << sender->metaObject()->method(signalIndex).signature() << ')';
 #endif
-    internalEventQueue.append(new QStateMachine::SignalEvent(sender, signalIndex, vargs));
+    postInternalEvent(new QStateMachine::SignalEvent(sender, signalIndex, vargs));
     processEvents(DirectProcessing);
 }
 
@@ -1825,6 +1867,8 @@ void QStateMachine::stop()
 }
 
 /*!
+  \threadsafe
+
   Posts the given \a event of the given \a priority for processing by this
   state machine.
 
@@ -1852,16 +1896,18 @@ void QStateMachine::postEvent(QEvent *event, EventPriority priority)
 #endif
     switch (priority) {
     case NormalPriority:
-        d->externalEventQueue.append(event);
+        d->postExternalEvent(event);
         break;
     case HighPriority:
-        d->internalEventQueue.append(event);
+        d->postInternalEvent(event);
         break;
     }
     d->processEvents(QStateMachinePrivate::QueuedProcessing);
 }
 
 /*!
+  \threadsafe
+
   Posts the given \a event for processing by this state machine, with the
   given \a delay in milliseconds. Returns an identifier associated with the
   delayed event, or -1 if the event could not be posted.
@@ -1893,12 +1939,15 @@ int QStateMachine::postDelayedEvent(QEvent *event, int delay)
 #ifdef QSTATEMACHINE_DEBUG
     qDebug() << this << ": posting event" << event << "with delay" << delay;
 #endif
+    QMutexLocker locker(&d->delayedEventsMutex);
     int tid = startTimer(delay);
     d->delayedEvents[tid] = event;
     return tid;
 }
 
 /*!
+  \threadsafe
+
   Cancels the delayed event identified by the given \a id. The id should be a
   value returned by a call to postDelayedEvent(). Returns true if the event
   was successfully cancelled, otherwise returns false.
@@ -1912,6 +1961,7 @@ bool QStateMachine::cancelDelayedEvent(int id)
         qWarning("QStateMachine::cancelDelayedEvent: the machine is not running");
         return false;
     }
+    QMutexLocker locker(&d->delayedEventsMutex);
     QEvent *e = d->delayedEvents.take(id);
     if (!e)
         return false;
@@ -1963,15 +2013,20 @@ bool QStateMachine::event(QEvent *e)
         int tid = te->timerId();
         if (d->state != QStateMachinePrivate::Running) {
             // This event has been cancelled already
+            QMutexLocker locker(&d->delayedEventsMutex);
             Q_ASSERT(!d->delayedEvents.contains(tid));
             return true;
         }
+        d->delayedEventsMutex.lock();
         QEvent *ee = d->delayedEvents.take(tid);
         if (ee != 0) {
             killTimer(tid);
-            d->externalEventQueue.append(ee);
+            d->delayedEventsMutex.unlock();
+            d->postExternalEvent(ee);
             d->processEvents(QStateMachinePrivate::DirectProcessing);
             return true;
+        } else {
+            d->delayedEventsMutex.unlock();
         }
     }
     return QState::event(e);
diff --git a/src/corelib/statemachine/qstatemachine_p.h b/src/corelib/statemachine/qstatemachine_p.h
index cf7a073..69b727d 100644
--- a/src/corelib/statemachine/qstatemachine_p.h
+++ b/src/corelib/statemachine/qstatemachine_p.h
@@ -58,6 +58,7 @@
 #include <QtCore/qcoreevent.h>
 #include <QtCore/qhash.h>
 #include <QtCore/qlist.h>
+#include <QtCore/qmutex.h>
 #include <QtCore/qpair.h>
 #include <QtCore/qset.h>
 #include <QtCore/qvector.h>
@@ -159,6 +160,13 @@ public:
     void unregisterAllTransitions();
     void handleTransitionSignal(QObject *sender, int signalIndex,
                                 void **args);    
+
+    void postInternalEvent(QEvent *e);
+    void postExternalEvent(QEvent *e);
+    QEvent *dequeueInternalEvent();
+    QEvent *dequeueExternalEvent();
+    bool isInternalEventQueueEmpty();
+    bool isExternalEventQueueEmpty();
     void processEvents(EventProcessingMode processingMode);
     void cancelAllDelayedEvents();
     
@@ -181,6 +189,8 @@ public:
     QSet<QAbstractState*> configuration;
     QList<QEvent*> internalEventQueue;
     QList<QEvent*> externalEventQueue;
+    QMutex internalEventMutex;
+    QMutex externalEventMutex;
 
     QStateMachine::Error error;
     QStateMachine::RestorePolicy globalRestorePolicy;
@@ -214,6 +224,7 @@ public:
     QHash<QObject*, QHash<QEvent::Type, int> > qobjectEvents;
 #endif
     QHash<int, QEvent*> delayedEvents;
+    QMutex delayedEventsMutex;
   
     typedef QEvent* (*f_cloneEvent)(QEvent*);
     struct Handler {
diff --git a/tests/auto/qstatemachine/tst_qstatemachine.cpp b/tests/auto/qstatemachine/tst_qstatemachine.cpp
index 346afc9..97057c6 100644
--- a/tests/auto/qstatemachine/tst_qstatemachine.cpp
+++ b/tests/auto/qstatemachine/tst_qstatemachine.cpp
@@ -206,6 +206,7 @@ private slots:
     void goToState();
 
     void task260403_clonedSignals();
+    void postEventFromOtherThread();
 };
 
 tst_QStateMachine::tst_QStateMachine()
@@ -4205,5 +4206,52 @@ void tst_QStateMachine::task260403_clonedSignals()
     QCOMPARE(t1->eventSignalIndex, emitter.metaObject()->indexOfSignal("signalWithDefaultArg()"));
 }
 
+class EventPosterThread : public QThread
+{
+    Q_OBJECT
+public:
+    EventPosterThread(QStateMachine *machine, QObject *parent = 0)
+        : QThread(parent), m_machine(machine), m_count(0)
+    {
+        moveToThread(this);
+        QObject::connect(m_machine, SIGNAL(started()),
+                         this, SLOT(postEvent()));
+    }
+protected:
+    virtual void run()
+    {
+        exec();
+    }
+private Q_SLOTS:
+    void postEvent()
+    {
+        m_machine->postEvent(new QEvent(QEvent::User));
+        if (++m_count < 10000)
+            QTimer::singleShot(0, this, SLOT(postEvent()));
+        else
+            quit();
+    }
+private:
+    QStateMachine *m_machine;
+    int m_count;
+};
+
+void tst_QStateMachine::postEventFromOtherThread()
+{
+    QStateMachine machine;
+    EventPosterThread poster(&machine);
+    StringEventPoster *s1 = new StringEventPoster("foo", &machine);
+    s1->addTransition(new EventTransition(QEvent::User, s1));
+    QFinalState *f = new QFinalState(&machine);
+    s1->addTransition(&poster, SIGNAL(finished()), f);
+    machine.setInitialState(s1);
+
+    poster.start();
+
+    QSignalSpy finishedSpy(&machine, SIGNAL(finished()));
+    machine.start();
+    QTRY_COMPARE(finishedSpy.count(), 1);
+}
+
 QTEST_MAIN(tst_QStateMachine)
 #include "tst_qstatemachine.moc"