Correct way to interrupt long running calculation inside a slot in another thread?

[CJha]

Hi, I am looking for a way to reliably abort some time taking calculations that I do in a worker thread and I am confused in terms of which approach to follow. I have a Master class object subclassed from QWidget which runs in the Main GUI Thread, a Worker class object subclassed from QObject and a WorkerThread object which is of type QThread. I create the Worker object in the constructor of Master and then use Worker->moveToThread(WorkerThread) and WorkerThread->start() in the constructor of Master. In my Worker object I have the following two public slots:

// a long running calculation inside a slot:
void Worker::doCalculation(const QVector<double>& vec)
{
    for(int ii = 0; ii < vec.length(); ++ii)
    {
        if(abortCalculation)
            return;
        else
        {
            // Do the calculation
            // Each iteration takes around 0.5 ms
            // Total: 1K to 1000K iteration in each call
        }
    }
}

// a slot to change the value of control variable:
void Worker::stopCalculation(bool isStop)
{
    abortCalculation = isStop;
}

The variable abortCalculation is of type bool and is defined in worker.h. The Master object calls the Worker::doCalculation(const QVector<double>& vec) from the main GUI thread to start the calculation at any time. I use Qt::QueuedConnection to execute this slot. Once in a while, the Master has to ask Worker to stop the calculation in between, and to terminate it prematurely I have provided the abortCalculation variable in the Worker object which it checks in each loop of the long-running calculation.

The question is how should I set the abortCalculation from Master, should I use:

• Qt::QueuedConnection to invoke stopCalculation() from Master?, or
• Qt::DirectConnection to invoke stopCalculation() from Master?, or
• Make abortCalculation public in Worker and directly change this variable from Master? Since I am only reading the value of abortCalculation in my slot doCalculation() to me it seems like this approach is the fastest and easiest approach. I can use QMutex to protect abortCalculation variable, but is that really necessary in this scenario?

[Christian Ehrlicher]

Don't use a connection type at all - Qt knows it better than you in 99,99% of all cases. And it will use QueuedConnection here which is correct.

[ollarch]

Hi,

You don't need any slot to do this. Calling "yourWorkerObject->stopCalculation(true)" is enough.

[Christian Ehrlicher]

@ollarch But still produces a race condition. So even it works because it's a simply bool it's not the correct way to do it.

[ollarch]

@Christian-Ehrlicher Why produces a race condition? The main thread changes the boolean variable and it let the worker thread exit. The main thread continues it execution after changing it.

[J.Hilk]

This post is deleted!

[JonB]

@ollarch said in Correct way to interrupt long running calculation inside a slot in another thread?:

@Christian-Ehrlicher Why produces a race condition? The main thread changes the boolean variable and it let the worker thread exit.

The main thread sets the variable at the same time as the thread is potentially running and reading that variable. This is a "race condition". @Christian-Ehrlicher is saying that, even if it works in this case because you're only setting a simple bool variable, this is not the right way to set/read a variable which is shared between threads (one writer, one reader). At minimum you should use a QMutex to be safe.

[ollarch]

@JonB I understand it, I'm only curious about it. I thought that the assignment was an atomic operation and so there was no problem. On non atomic assignments, yes, a Mutex is needed.
Thanks for explanation.

[jsulm]

@ollarch Assignment is not atomic. For basic types like int you can make it atomic, but it is up to you as developer.

[CJha]

@jsulm Hi, by just using std::atomic<bool> abortCalculation should be enough right? Then I can access this variable from any thread without needing to use QMutex or Signal-Slot connection?

[CJha]

@Christian-Ehrlicher Wouldn't Qt::QueuedConnection wait for my doCalculation() slot to finish before invoking the stopCalculation() if I am calling both of these slots from Master?

[JonB]

@CJha
If you are waiting for doCalculation() to finish before letting stopCalculation() run, it will never abort the loop.....

[CJha]

@JonB Thanks, but my question is will the call to stopCalculation() wait for doCalculation() to finish if I am using Qt::QueuedConnection and calling these slots from Master?

[J.Hilk]

@CJha no, the slot call would be invoked when the eventloop starts running again, and it won't as long as your function did not return.

The only way would be with a forced direct connection, than the slot is executed in the other thread. But you will have to mutex lock inside the slot than.

[CJha]

@J-Hilk Thanks a lot for clarifying this, I working on a solution now but it takes a long time to make the code and I was constantly wondering about this till now.

[jsulm]

@CJha said in Correct way to interrupt long running calculation inside a slot in another thread?:

std::atomic<bool> abortCalculation

Yes, this should do it

[CJha]

Hi everyone, currently I am working on a solution to check which method works best. But since Master is a huge class it is taking me some time to finish it for final testing. I have changed my method, now I am using a local QEventLoop in Master to wait for an aborted() signal from my Worker object.

The code outline for Master to control Worker is as follows:

// Masters slot controlling worker's behaviour
// This slot is called by a QPushButton in GUI
void Master::startCalculation()
{
    if(workerRunning) // std::atomic<bool> workerRunning; - defined in master.h
    {
        // If yes, start an event loop and wait for aborted signal
        QEventLoop loop;
        connect(workerObject, &Worker::aborted, &loop, &QEventLoop::quit);
        workerObject->abortCalculation = true;
        loop.exec();
    }
    // now start new calculation
    workerObject->doCalculation(dataVector); // dataVector is QVector<double>
}

The Worker code is changed now:

// a long running calculation inside a slot:
void Worker::doCalculation(const QVector<double>& vec)
{
    // Tell master that worker is starting
    masterObject->workerRunning = true;

    // Start the calculation part
    for(int ii = 0; ii < vec.length(); ++ii)
    {
        if(abortCalculation){ // std::atomic<bool> abortCalculation; - defined in worker.h
            emit aborted();
            return;
        }
        else
        {
            // Do the calculation
            // Each iteration takes around 0.5 ms
            // Total: 1K to 1000K iteration in each call
        }
    }

    // Tell master worker has stopped
    masterObject->workerRunning = false;
}

To me it appears that the above solution will be the best one. But I still have one small doubt: Does starting a new QEventLoop inside a slot of main GUI thread and waiting for it to quit() stop the event loop of main GUI thread? As far as my understanding goes, I think that will be the case, if so, is there a way I can wait for aborted() signal inside a main GUI slot without blocking the entire main GUI thread?

[JonB]

@CJha
If workerRunning is true your startCalculation() blocks and waits until Worker is aborted, and only then sets off doCalculation(). And if Worker does not call aborted() that will be when Hell freezes over. Is this really your intended architecture?? I'm lost with what you are up to....

[CJha]

@JonB Thanks, yes this is my intended architecture. In my Worker I am generating a line plot on a QImage which it sends to Master. The Master is a subclassed QWidget where I display this QImage as a plot in a certain area. My architecture is this way because I have enabled multiple interactions with the plot in Master using the mouse and keyboard interactions, such that users can zoom and scroll. However, since the data to be plotted is quite big I need to have a way to abort the generation of QImage inside Worker otherwise the zoom and scroll behaviour will lag enormously.

[JonB]

@CJha
Doesn't sound right to me. If your Worker finishes without going emit aborted(); you never exit your loop.exec() and are stuck there, like I said, till The Universe comes to an end.

I don't understand, so I will just say this. I have a funny feeling that what you want is for the main thread to check for worker aborted intermittently, while remaining UI-responsive and checking for aborted, which requires signal processing. If that is so you have two possible approaches:

• Call processEvents() each time round its loop. Signals will be processed at that point.
• Change so you do not have a tight loop around each element in the vector. Instead have a QTimer() and on each timeout process some number of the remaining items in the vector, noting where you got to.

I leave it to you....