Unsolved Correct way to interrupt long running calculation inside a slot in another thread?
-
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 fromQWidget
which runs in the Main GUI Thread, aWorker
class object subclassed fromQObject
and aWorkerThread
object which is of typeQThread
. I create theWorker
object in the constructor ofMaster
and then useWorker->moveToThread(WorkerThread)
andWorkerThread->start()
in the constructor ofMaster
. In myWorker
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 typebool
and is defined in worker.h. TheMaster
object calls theWorker::doCalculation(const QVector<double>& vec)
from the main GUI thread to start the calculation at any time. I useQt::QueuedConnection
to execute this slot. Once in a while, theMaster
has to askWorker
to stop the calculation in between, and to terminate it prematurely I have provided theabortCalculation
variable in theWorker
object which it checks in each loop of the long-running calculation.The question is how should I set the
abortCalculation
fromMaster
, should I use:Qt::QueuedConnection
to invokestopCalculation()
fromMaster
?, orQt::DirectConnection
to invokestopCalculation()
fromMaster
?, or- Make
abortCalculation
public inWorker
and directly change this variable fromMaster
? Since I am only reading the value ofabortCalculation
in my slotdoCalculation()
to me it seems like this approach is the fastest and easiest approach. I can useQMutex
to protectabortCalculation
variable, but is that really necessary in this scenario?
-
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.
-
Hi,
You don't need any slot to do this. Calling "yourWorkerObject->stopCalculation(true)" is enough.
-
@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.
-
@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.
-
This post is deleted! -
@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 aQMutex
to be safe. -
@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. -
@ollarch Assignment is not atomic. For basic types like int you can make it atomic, but it is up to you as developer.
-
@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 useQMutex
or Signal-Slot connection? -
@Christian-Ehrlicher Wouldn't
Qt::QueuedConnection
wait for mydoCalculation()
slot to finish before invoking thestopCalculation()
if I am calling both of these slots fromMaster
? -
@CJha
If you are waiting fordoCalculation()
to finish before lettingstopCalculation()
run, it will never abort the loop..... -
@JonB Thanks, but my question is will the call to
stopCalculation()
wait fordoCalculation()
to finish if I am usingQt::QueuedConnection
and calling these slots fromMaster
? -
@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.
-
@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.
-
@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
-
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 localQEventLoop
inMaster
to wait for anaborted()
signal from myWorker
object.The code outline for
Master
to controlWorker
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 toquit()
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 foraborted()
signal inside a main GUI slot without blocking the entire main GUI thread? -
@CJha
IfworkerRunning
is true yourstartCalculation()
blocks and waits untilWorker
is aborted, and only then sets offdoCalculation()
. And ifWorker
does not callaborted()
that will be when Hell freezes over. Is this really your intended architecture?? I'm lost with what you are up to.... -
@JonB Thanks, yes this is my intended architecture. In my
Worker
I am generating a line plot on aQImage
which it sends toMaster
. TheMaster
is a subclassedQWidget
where I display thisQImage
as a plot in a certain area. My architecture is this way because I have enabled multiple interactions with the plot inMaster
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 ofQImage
insideWorker
otherwise the zoom and scroll behaviour will lag enormously. -
@CJha
Doesn't sound right to me. If yourWorker
finishes without goingemit aborted();
you never exit yourloop.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....
- Call