Thread pausing/resuming/canceling with Qt

I have written this code to be able to suspend (or to cancel) a worker executed in a separate thread in Qt.

To do it, I have used an instance QWaitCondition and QMutex.

#ifndef MYWORKER_H
#define MYWORKER_H

#include
#include
#include

class MyWorker : public QObject
{
Q_OBJECT
public:
explicit MyWorker(QObject *parent = 0);
~MyWorker();
void restart();
void pause();

signals:
void finished();

public slots:
void doWork();
void cancelWork();

private:
QMutex m_continue;
QWaitCondition m_pauseManager;
bool m_cancelRequested;
bool m_pauseRequired;
};

#endif // MYWORKER_H


Here is the code of myworker.cpp

#include "myworker.h"

#include 

MyWorker::MyWorker(QObject *parent) :
    QObject(parent),
    m_cancelRequested(false),
    m_pauseRequired(false)
{
}

MyWorker::~MyWorker()
{
}

void MyWorker::restart()
{
    this -> m_pauseRequired = false;
    this -> m_pauseManager.wakeAll();

}

void MyWorker::pause()
{
    this -> m_pauseRequired = true;
}

void MyWorker::doWork()
{
    // Write your code here
    for(int i = 0; i  m_cancelRequested()) {
            break;
        }
    }

    // Write your code above"

    emit finished();
}

void MyWorker::cancelWork()
{
    this -> restart();
    this -> m_cancelRequested = true;
}

Here is the snippet to use this code :

this -> m_thread = new QThread();
this -> m_worker = new MyWorker();
this -> m_worker-> moveToThread(this -> m_thread);
connect(m_thread, SIGNAL(started()), m_worker, SLOT(doWork()));
connect(m_worker, SIGNAL(finished()), m_thread, SLOT(quit()));

this -> m_thread -> start();

// To suspend the work
this -> m_worker -> pause();

// To stop the work
this -> m_worker -> cancelWork();

Do you have any advice on improving this code, or do you have a better solution?

The code you have is unsafe, because pause() and cancelWork() are called from outside the worker's thread, and they use no locking when modifying the worker's members.

In general, prefer to interact with objects in other threads by invoking their slots via queued connections, unless there's a pressing reason to act immediately. This means you don't need any of the synchronization facilities inside the worker. This simplifies the coding, and makes it easier to test.

To do that, the worker needs to periodically process events. I prefer to put this in a function of its own:

bool isCancelled() {
    auto dispatcher = QThread::currentThread()->eventDispatcher();
    if (!dispatcher) return false;
    dispatcher->processEvents();
    return m_cancelRequested;
}

In the work method, this can be called every time you want to poll for cancellation.

You need some protection against re-entering doWork() from this event queue. One way to manage this is with a tiny state-machine:

private:
    enum State { IDLE, RUNNING, PAUSED };
    State state = IDLE;

Used like this:

void MyWorker::doWork()
{
    if (state == RUNNING)
        return;

    state = RUNNING;
    emit started();

Here's my complete, tested version - no locks or atomics, all the synchronization is managed by Qt:

#include 
#include 
#include 
#include 

class Worker : public QObject
{
    Q_OBJECT

public:
    explicit Worker(QObject *parent = 0)
        : QObject(parent)
    {
    }

signals:
    void started();
    void finished();

public slots:
    void pause()
    {
        auto const dispatcher = QThread::currentThread()->eventDispatcher();
        if (!dispatcher) {
            qCritical() processEvents(QEventLoop::WaitForMoreEvents);
        } while (state == PAUSED);
    }

    void resume()
    {
        if (state == PAUSED) {
            state = RUNNING;
            qDebug() eventDispatcher();
        if (!dispatcher) {
            qCritical() processEvents(QEventLoop::AllEvents);
        return state == IDLE;
    }

};

class MyWorker : public Worker
{
    Q_OBJECT

public:
    explicit MyWorker(QObject *parent = 0)
        : Worker(parent)
    {
    }

    void doWork()
    {
        if (state == PAUSED)
            // treat as resume
            state = RUNNING;

        if (state == RUNNING)
            return;

        state = RUNNING;
        qDebug() 
int main(int argc, char **argv)
{
    QCoreApplication app(argc, argv);

    QThread thread;
    auto worker = new MyWorker();
    worker->moveToThread(&thread);
    QObject::connect(&thread, &QThread::started, worker, &MyWorker::doWork);
    QObject::connect(worker, &MyWorker::finished, worker, &QObject::deleteLater);
    QObject::connect(worker, &QObject::destroyed, &thread, &QThread::quit);
    QObject::connect(&thread, &QThread::finished, &app, &QCoreApplication::quit);

    thread.start();

    // To saved defining more signals, I'll just use invoke() here.
    // Normally, we connect some "driver" object's signals to the
    // pause/resume/cancel slots.
    auto const m = worker->metaObject();
    QThread::sleep(1);
    m->invokeMethod(worker, "pause");
    QThread::sleep(4);
    m->invokeMethod(worker, "resume");
    QThread::sleep(1);
    m->invokeMethod(worker, "cancel");

    app.exec();
}

Sorry this is a bit rushed; I hope the principle is clear.

StackExchange Code Review Q#26724, answer score: 9