Terminate QThread correctly

Andrea

Hi everybody,

I've a doubt about how to correctly close a QThread.
I've read the QThread Basics docs and also had a look on StarckOverflow and the Qt forum, but still having the doubt.

I've implemented many htread-worker threads, and they all work correctly. The only problem is when I delete the thread: I always get "QThread: destroyed while thread is still running".
Putting a "wait()" call after the "terminate()" one, never exit since it remains pending waiting for the thread termination.

Here a bit of my code:

/*   THIS IS THE CREATION OF THE THREAD CONTROLLER AND THE WORKER */
QThread *threadController = new QThread(this);

//	ThreadWorker is my custom class
ThreadWorker* threadWorker = new ThreadWorker(  );
threadWorker->moveToThread( threadController );

connect( threadController , SIGNAL(started()), threadWorker, SLOT(Work()), Qt::QueuedConnection );

threadController ->start();

Here the implementation of the custom class

/*   ThreadWorker.h  */
class ThreadWorker : public QObject
{
    Q_OBJECT

public:
    ThreadWorker();
    ~ThreadWorker();

public slots:
    void Work();
    void onAbort();

private:
    bool thread_exit;
};


/*   ThreadWorker.cpp    */
//	Constructor
ThreadWorker::ThreadWorker()
{
    this->thread_exit = false;
}

//	Destructor
ThreadWorker::~ThreadWorker()
{
}

//	"Main" function
void ThreadWorker::Work() 
{
	//	Thread loop
	while( !this->thread_exit )
	{
		//  Wait
		sleep(1);

		//	Do some actions..

		//  Process application events
		QCoreApplication::processEvents( QEventLoop::AllEvents );
	}
}

//	Stop thread
void ThreadWorker::onAbort()
{
	//  Exit thread loop
	this->thread_exit = true;
}

I'm using Qt 4.8 on a iMX6SX board with Yocto Dizzy distro.

Many thanks
Andrea

mrjj

Hi
How do you call
ThreadWorker::onAbort() ?

if not via signal & slot then you might need to wrap
thread_exit in a std::atomic<bool> to avoid race conditions.

SGaist

Hi,

If you have an infinite loop, you should rather have a stop method that allows you to exit the loop cleanly. Terminating threads is never a good idea and should only be a last resort solution.

Andrea

@mrjj the onAbort() slot is called via signal, with QueuedConnection, since the emitter and receiver live in different threads.
@SGaist there is not a "stop" method, but the "onAbort()" method does same logical operation: it just sets the bool condition checked in the loop to exit from this latter one. Maybe I could be wrong, but to me it seems this is a clean way to exit thread loop, isn't it?
Mmm..but when I'm going to close the application, terminate a thread seems to be a good way to follow before closing the whole application or should I just close the application a let the OS "manage" threads created? Thanks

jsulm

@Andrea After calling onAbort() do you actually wait for the threads to finish? See http://doc.qt.io/qt-5.9/qthread.html#wait

J.Hilk

@Andrea

Usually you call quit to quit a thread:
Try this.

connect(qApp, &QApplication::aboutToQuit,   threadController, [=]{
    threadController->quit();
    threadController->wait();
});

Andrea

@jsulm if I put a wait() after "calling" onAbort() method (via signal-slot), hte program hang waiting the thread to finish...but this never happens. I'm sure the thread exits its while loop (I've used qDebug() to check this), but calling wait function of thread controller never returns.

J.Hilk

@Andrea
wait() will return when either of these 2 conditions are met:

The thread associated with this QThread object has finished execution (i.e. when it returns from run()). This function will return true if the thread has finished. It also returns true if the thread has not been started yet.
time milliseconds has elapsed. If time is ULONG_MAX (the default), then the wait will never timeout (the thread must return from run()). This function will return false if the wait timed out.

the default time is ULONG_MAX which is 4294967295 ms -> roughly 50 days

you never call quit on threadController, onAbort only effects your worker object, not the thread wrapper.

SGaist

Sorry, I missed the implementation of onAbort while looking at the code.

Note that that name is a bit confusing. It rather sounds like something that would be called when the thread has aborted.

IIRC, boolean variable used like that might be "optimised". I'd replace it with a QAtomicInt.

JKSH

@Andrea said in Terminate QThread correctly:

@jsulm if I put a wait() after "calling" onAbort() method (via signal-slot), hte program hang waiting the thread to finish...but this never happens. I'm sure the thread exits its while loop (I've used qDebug() to check this), but calling wait function of thread controller never returns.

Hi @Andrea,

You stopped the loop in ThreadWorker, but you didn't stop the loop in QThread.

As @J-Hilk said, you need to call threadController->quit() too.

@Andrea said in Terminate QThread correctly:

I've read the QThread Basics docs and also had a look on StarckOverflow and the Qt forum, but still having the doubt.

Also read through http://doc.qt.io/qt-5/qthread.html#details and http://doc.qt.io/qt-5/threads-technologies.html. There are 2 ways to use QThread:

Create a worker object
Subclass QThread and override QThread::run().

If you want your thread to receive signals, you should use a worker object. However, if you want to run an infinite loop (while( !this->thread_exit ) { /*...*/ }) and you don't need signals/slots, then it's simpler to subclass QThread.

Judging by your code example, I don't think you need a worker object. Also, if you subclass QThread, the thread will quit when your while loop stops.

Buckwheat

Hi @Andrea

WOW! Infinite loop to just sleep and process events. Why not just start a timer of PreciseTimer and allow the events to flow freely in the thread? Since you want it to run as fast as possible you can even set the timeout to 0 to run freely when there are no events.

You can then look at thread->requestInterruption () and the thread will comply! Just make sure if you are doing something in a loop in the timer callback you check thread ()->isInterruptionRequested (). And best of all... NO CHECKING THE EVENT QUEUE!

There is a nice writeup about proper thread use at: https://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/

You are basically there with your code. Just the loop is wasteful.