Shutting down threads prematurely

[Lumbricus]

hey guys,

my app does calculations in backround threads which are contained in a worker object. The calculations performed are long and sometimes i will want to shut down the calculations and restart them. I wanted to let the main gui connect to the worker and tell it to terminate its current thread but somehow i cant get it to work.

from mainwindow.cpp:

creating threads and connecting them + start thread



for (int id=1;id<=guidata.numbers_of_workerthreads;id++)
{

QThread *thread = new QThread;

Worker *worker = new Worker(&guidata,ui,id,&m_c,&mutex_file,&mutex_count);

worker->moveToThread(thread);

//connect events from worker<-> threads
connect(worker, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
connect(thread, SIGNAL(started()), worker, SLOT(comets_to_use()));
connect(this, SIGNAL(shut_down_threads()), worker ,SLOT(on_shut_down_threads())); //<--- how im trying to access thread/worker from gui
connect(worker, SIGNAL(finished()), thread, SLOT(quit()));
connect(worker, SIGNAL(finished()), worker, SLOT(deleteLater()));
connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));

thread->start();
}
}


this button is pressed to initiate the threads to shut down;



//stop button
void MainWindow::on_pushButton_9_clicked()
{

qDebug() << "shutdown emitted";
emit shut_down_threads();


from worker.cpp:
(i know terminate is a bad practice this is just to get the initial shutdown to work)



void Worker::on_shut_down_threads()
{
qDebug() << "terminating thread " << QString::number(thread_id);
QThread::currentThread()->terminate();
}


sadly this is not stopping my calculations. Any pointers where i went wrong? im kinda lost! thx boys.

[anda_skoa]

Cross-Thread Signal/Slot connections (or more precisely Qt::QueuedConnection type connections) require a running event loop in the receiver object's thread.

Theoretically that is the case, as QThread::run's base implementation runs an event loop.
However, if comets_to_use() slot is the long running operation, then the event loop isonly ever executing that slot, the shutdown will be queued until that first slot ends.

What you need is a method on the worker object that is being called by the main thread, e.g. using signal/slot but using a Qt::DirectConnection.
Obviously you can't use QThread::currentThread() in that method since that will be executed by the main thread :-)

Cheers,
_

[Lumbricus]

Thx anda for pointing me in the right direction. I was not aware i'm blocking the eventloop of my threads with that long-ass slot.

So i tried your approach with the use of a direct connection.



connect(this, SIGNAL(shut_down_threads()), worker ,SLOT(on_shut_down_threads()),Qt::DirectConnection );


this works and instantly calls the slot in the worker. when i press my stop button. BUT

output:

shutdown emitted
terminating thread "1"
terminate called without an active excemption
The program has unexpectedly finished.

so i tried changing the command from terminate() of the thread to prematurely



emit finished();


but that had no effect. I thought i might just be able to call the normal destruction chain when my worker finishes, but no luck.

next try was to change the terminate() to:



QThread::currentThread()->quit();


output:
shutdown emitted
terminated thread "1"
terminated thread "2"
... /imex exited with code 0

So the currentThread() statement is called in the "worker threads" but it also quits my main thread. Now i'm confused. I saw something you posted in 2015 where you were saying something about directconnections calling the slot in the thread where it was emitted...is this the case her? Thx anda you've been helping me a lot.

[anda_skoa]

I was not aware i'm blocking the eventloop of my threads with that long-ass slot.

Any thread can only execute one code path at any given time.



this works and instantly calls the slot in the worker. when i press my stop button. BUT

output:

shutdown emitted
terminating thread "1"
terminate called without an active excemption
The program has unexpectedly finished.

Maybe you forgot to replace the QThread::currentThread() with somethng to access the worker thread?

so i tried changing the command from terminate() of the thread to prematurely





emit finished();


but that had no effect. I thought i might just be able to call the normal destruction chain when my worker finishes, but no luck.

You are using delayed deletion via deleteLater(), so again after the thread's event loop had a chance to process events.




next try was to change the terminate() to:



QThread::currentThread()->quit();


In a slot executed by the worker thread, right?
Not in on_shut_down_threads(), right?



So the currentThread() statement is called in the "worker threads" but it also quits my main thread.

How did you do the thread transition?



I saw something you posted in 2015 where you were saying something about directconnections calling the slot in the thread where it was emitted...is this the case her?

Yes, I even wrote that in my other reply here, last sentence.

My recommendation would be to derive from QThread and reimplement run() instead.
Much more obvious which thread does what.

Cheers,
_

[Lumbricus]

Hey,

on_shut_down_threads() is a slot of the worker that was moved to the new thread. So it lives in the worker thread. So accessing currentThread() <- is the worker thread.

I have like 15 methods/slots of the worker to do different kind of work. I would have to reimplement a derived thread run() for all of them right?

Is there a way to unblock the eventloop of the thread...like just making the slot call a function and then carry on looping?

[anda_skoa]

on_shut_down_threads() is a slot of the worker that was moved to the new thread. So it lives in the worker thread. So accessing currentThread() <- is the worker thread.

Ah, I thought you had connected that slot with a Qt::DirectConnection.



I have like 15 methods/slots of the worker to do different kind of work. I would have to reimplement a derived thread run() for all of them right?

Ah, so you have many methods like "comets_to_use()" and you call them from different trigger signals form the main thread?



Is there a way to unblock the eventloop of the thread...like just making the slot call a function and then carry on looping?
You can get the thread's event dispatcher and call its processEvents() now and then, but be aware that this leads to re-entrancy-like behavior even withing a single thread.

Cheers,
_

[Lumbricus]

Ah, I thought you had connected that slot with a Qt::DirectConnection.


this is the code posted earlier, signal is connected to worker object slot, in worker thread;


connect(this, SIGNAL(shut_down_threads()), worker ,SLOT(on_shut_down_threads()),Qt::DirectConnection );


Ah, so you have many methods like "comets_to_use()" and you call them from different trigger signals form the main thread?

yes i have different methods like comets_to_use.

In the beginning I had a different approach i tried. It was keeping the pointers to the created threads and trying to tell the threads do exit from the main thread. I had something like this:



for (int i=0,i<number of threads,i++)
{
QThread *thread = new QThread;
//save pointer
active_threads[i] = thread;

//then do worker object set up and move it to thread and start thread.
....
}

and then when i click the button run something like this:



emit active_threads[i]->quit();
//
active_threads[i]->terminate;


it didn't seem to work would that be a better approach? or am i making an obvious mistake? Is this also blocked by the long executing slot in the worker?

[anda_skoa]

this is the code posted earlier, signal is connected to worker object slot, in worker thread;


connect(this, SIGNAL(shut_down_threads()), worker ,SLOT(on_shut_down_threads()),Qt::DirectConnection );

Ok, now I am afraid I can't follow.
You call the slot via a direct connection, so you are no longer using QThread::currentThread() in that slot, right?
So are you somehow making the other thread execute some code or are you using a different approach to get the thread pointer?




yes i have different methods like comets_to_use.

Which you call from the main thread once comets_to_use is done?





emit active_threads[i]->quit();


Well, quit() isn't a signal and if ti where you shouldn't emit another object's signals, but ok, that makes the thread's event loop stop, so the run() will return once the worker doesn't execute any slot anymore.




it didn't seem to work would that be a better approach?

That at least makes you call terminate on the right thread.
Not that you would ever want to call terminate() of course.

Cheers,
_

[Lumbricus]

My bad cut the post to short, i will try to reexplain:

I make my worker object and create a thread, then move the object to the thread and start the thread.

the started signal of the thread is connected to the slot of the worker i want to run e.g (comets_to_use() ).

the worker contains a slot called on_shut_down_threads(). inside that i was using QThread::currentThread() to access the thread that was executing the worker. So i press a button and that emits a signal with is connected to this worker slot.

So this worker is in the worker thread:



void Worker:on_shut_down_threads()
{
QThread::currentThread()->exit();
}


Since i want to close down the calculations in the slot comets_to_use() before it has finished I'm trying to find a way shut down that thread/or stop executing that worker slot.

yes emitting the signal ->quit() was dumb.

i still dont see why my thread does not close if i run



active_threads[i]->terminate();

or:


active_threads[i]->exit();


from the main thread thow?

[anda_skoa]

the worker contains a slot called on_shut_down_threads(). inside that i was using QThread::currentThread() to access the thread that was executing the worker.

QThread::currentThread() returns the current thread.
If that is in the slot called by the main thread, then the main thread is the current thread.



So i press a button and that emits a signal with is connected to this worker slot.

So it depends on how the connection() was created.
In an AutoConnection the receiver object's thread executes the slot, in a QueuedConnection the signal emitting thread executes the slot.

So if you have a QueuedConnection as you claim, then accessing the current thread will never access the worker thread as it is not the current thread.

i still dont see why my thread does not close if i run





active_threads[i]->terminate();

or:


active_threads[i]->exit();


QThread::exit() is basically the same as QThread::quit(), both make the thread's event loop end.

Lets go back to your need to that kind of threading approach.
Aside from the slot that is run on start, what other slots does the worker have and how does the main thread trigger them?

Cheers,
_

[Lumbricus]

okai back to basics:



for (int id=1;id<=guidata.numbers_of_workerthreads;id++)
{

QThread *thread = new QThread;

Worker *worker = new Worker(&guidata,ui,id,&m_c,&mutex_file,&mutex_count);

worker->moveToThread(thread);

//connect events from worker<-> threads
connect(worker, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
connect(thread, SIGNAL(started()), worker, SLOT(comets_to_use())); // when i start the thread it automaticly runs my slot that i specify
connect(this, SIGNAL(shut_down_threads()), worker ,SLOT(on_shut_down_threads()));
connect(worker, SIGNAL(finished()), thread, SLOT(quit()));
connect(worker, SIGNAL(finished()), worker, SLOT(deleteLater()));
connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));

thread->start();
}
}


this is how i start a specific method in the worker other methods like my calc_stream_allcomets() ... would get exchanged with comets_to_use(); Depending on what button is clicked on the gui the worker slot that is executed on start is varied.

As i said the calculations can be up to 1-2 days and sometimes i want to shut down the threads earlier e.g when you notice you made an input mistake of somekind.

[anda_skoa]

Ok, so depending on what you want to do you have a different slot of the worker connected to the thread's started() signal, but in all cases one worker object is only ever used for one operation and either cancelled or fully executed and then discarded.

Ok, then comets_to_use() and any of the other operations that need to be interruptible need check whether they need to stop.
And the slot connected to shut_down_threads() needs to be connected with a DirectConnection and needs to change whatever the worker does to check for stopping.

One option would be to check the worker's thread isInterruptionRequested() and use the worker's thread requestInterrruption() to switch to stopping.
The worker would then also make the thread quit its event loop right before it exits the processing function.

Cheers,
_

[Lumbricus]

That sounds like a good approach to me. I'm still unclear how i would stop the worker from doing what it is doing to check if an interruption is wanted.

What do I put in the worker slot that is connected? How would I change the worker from "working on my stuff". This is the Worker slot that is connected by direct connection to the main thread.



void Worker::on_shut_down_threads()
{
if("workerthread_pointer?"->isInterruptionRequested())
{
??
}

}


Am i Understanding u right? I think a little pseudo code or code would help me understand

[anda_skoa]

No, in that slot you request the interruption.


void Worker::on_shut_down_threads()
{
thread()->requestedInterruption();
}


The method that does the long calculation needs regularily to check for interruption.

Cheers,
_

[Lumbricus]

Okai anda,

i think i've nearly understood, thx for your patience!

In my worker slot i now have a check that is run through every 2sek. It checks if there is an interruption request from the main thread. The interrupt request is passed through with a direct connection. This is working at the moment. so in this part of the code how do i shut down the thread?


this is in the executing worker slot (comets_to_use()) in the worker thread:



//...worker doing stuff...

//comes past here every 2 sek

if (interruptRequested)
{
how do i shut down the thread from here?
}




the only way i got it to work was to put this request at the beginning of all my nested loops and break; out of them so i quickly reach the end of the slot. then the thread processes my finished() signal and destructs normaly. is there a nicer way to do this?

[anda_skoa]

You can nest the calls



void Worker::comets_to_use()
{
comets_to_use_internal();
thread()->quit();
}

That way you can just return from comets_in_use_internal() whenever you want to end.

Cheers,
_