How to Safely Destruct a Qthread

How to safely destruct a QThread?

The Safe Thread

In C++, the proper design of a class is such that the instance can be safely destroyed at any time. Almost all Qt classes act that way, but QThread doesn't.

Here's the class you should be using instead:

// Thread.hpp
#include <QThread>
public Thread : class QThread {
  Q_OBJECT
  using QThread::run; // This is a final class
public:
  Thread(QObject * parent = 0);
  ~Thread();
}

// Thread.cpp
#include "Thread.h"
Thread::Thread(QObject * parent): QThread(parent)
{}

Thread::~Thread() {
  quit();
  #if QT_VERSION >= QT_VERSION_CHECK(5,2,0)
  requestInterruption();
  #endif
  wait();
}

It will behave appropriately.

QObject Members Don't Need to Be on the Heap

Another problem is that the Worker object will be leaked. Instead of putting all of those objects on the heap, simply make them members of MyClass or its PIMPL.

The order of member declarations is important, since the members will be destructed in the reverse order of declaration. Thus, the destructor of MyClass will, invoke, in order:

1. m_workerThread.~Thread() At this point the thread is finished and gone, and m_worker.thread() == 0.

2. m_worker.~Worker Since the object is threadless, it's safe to destroy it in any thread.

3. ~QObject

Thus, with the worker and its thread as members of MyClass:

class MyClass : public QObject {
  Q_OBJECT
  Worker m_worker;          // **NOT** a pointer to Worker!
  Thread m_workerThread;    // **NOT** a pointer to Thread!
public:
  MyClass(QObject *parent = 0) : QObject(parent),
  // The m_worker **can't** have a parent since we move it to another thread.
  // The m_workerThread **must** have a parent. MyClass can be moved to another
  // thread at any time.
    m_workerThread(this)
  {
    m_worker.moveToThread(&m_workerThread);
    m_workerThread.start();
  }
};

And, if you don't want the implementation being in the interface, the same using PIMPL

// MyClass.hpp
#include <QObject>
class MyClassPrivate;
class MyClass : public QObject {
  Q_OBJECT
  Q_DECLARE_PRIVATE(MyClass)
  QScopedPointer<MyClass> const d_ptr;
public:
  MyClass(QObject * parent = 0);
  ~MyClass(); // required!
}

// MyClass.cpp
#include "MyClass.h"
#include "Thread.h"

class MyClassPrivate {
public:
  Worker worker;          // **NOT** a pointer to Worker!
  Thread workerThread;    // **NOT** a pointer to Thread!
  MyClassPrivate(QObject * parent);
};

MyClassPrivate(QObject * parent) :
  // The worker **can't** have a parent since we move it to another thread.
  // The workerThread **must** have a parent. MyClass can be moved to another
  // thread at any time.
    workerThread(parent)
{}

MyClass::MyClass(QObject * parent) : QObject(parent),
  d_ptr(new MyClassPrivate(this))
{
  Q_D(MyClass);
  d->worker.moveToThread(&d->workerThread);
  d->workerThread.start();
}

MyClass::~MyClass()
{}

QObject Member Parentage

We now see a hard rule emerge as to the parentage of any QObject members. There are only two cases:

1. If a QObject member is not moved to another thread from within the class, it must be a descendant of the class.

2. Otherwise, we must move the QObject member to another thread. The order of member declarations must be such that the thread is to be destroyed before the object. If is invalid to destruct an object that resides in another thread.

It is only safe to destruct a QObject if the following assertion holds:

Q_ASSERT(!object->thread() || object->thread() == QThread::currentThread())

An object whose thread has been destructed becomes threadless, and !object->thread() holds.

One might argue that we don't "intend" our class to be moved to another thread. If so, then obviously our object is not a QObject anymore, since a QObject has the moveToThread method and can be moved at any time. If a class doesn't obey the Liskov's substitution principle to its base class, it is an error to claim public inheritance from the base class. Thus, if our class publicly inherits from QObject, it must allow itself to be moved to any other thread at any time.

The QWidget is a bit of an outlier in this respect. At the very minimum, it should have made the moveToThread a protected method.

For example:

class Worker : public QObject {
  Q_OBJECT
  QTimer m_timer;
  QList<QFile*> m_files;
  ...
public:
  Worker(QObject * parent = 0);
  Q_SLOT bool processFile(const QString &);
};

Worker::Worker(QObject * parent) : QObject(parent),
  m_timer(this)  // the timer is our child
  // If m_timer wasn't our child, `worker.moveToThread` after construction
  // would cause the timer to fail.
{}

bool Worker::processFile(const QString & fn) {
  QScopedPointer<QFile> file(new QFile(fn, this));
  // If the file wasn't our child, `moveToThread` after `processFile` would
  // cause the file to "fail".
  if (! file->open(QIODevice::ReadOnly)) return false;      
  m_files << file.take();
}

Properly delete QThread

The main issue is that you are deleting the thread object (ie the QThread) while the thread of execution is running. In your threadFinished(), which is actually task finished, you need to do :

 thread->quit();
 thread->wait();
 thread->deleteLater();
 reader->deleteLater();

and remove the finished -> deleteLater connections.

Is it safe to call terminate() and quit() manually from a destructor of a class that was derived from QThread?

Using QThread::terminate() can lead to memory corruption, since the thread is just terminated without its knowledge, it can be doing anything while it gets terminated:

Warning: This function is dangerous and its use is discouraged. The thread can be terminated at any point in its code path. Threads can be terminated while modifying data. There is no chance for the thread to clean up after itself, unlock any held mutexes, etc. In short, use this function only if absolutely necessary.

To safely terminate a QThread, You need to have a way to tell the thread that it has to terminate, and when the thread gets that, it should return from its run() implementation as soon as possible. Qt provides two ways to do this:

• If your thread runs an event loop (i.e. You don't override run(), Or if you call exec() in your custom run() implementation), You can call QThread::quit()/QThread::exit() from any thread. This will cause the thread event's loop to return as soon as it finishes processing current events. There is no data corruption, as current processing doesn't get terminated.
• If your thread does not run an event loop, You can use QThread::requestInterruption() from any other thread to tell the thread that it should stop. But you have to handle that in your implementation of run() using isInterruptionRequested()(otherwise, calling requestInterruption() will do nothing).

Note:

If you are using any of the above methods to stop your QThread in its destructor, You have to make sure that the thread is no longer running after the QThread object gets destructed, You can do that by calling QThread::wait() after using quit()/requestInterruption().

Have a look at this answer for a similar implementation of a QThread subclass.

Qt: qthread destroyed while thread is still running during closing

You should note that if you have a loop running in a function of your thread, you should explicitly end it in order to properly terminate the thread.

You can have a member variable in your class named finishThread which should be set to true when the application is going to close. Just provide a slot in which you set the value for finishThread. When you want to terminate the thread emit a signal that is connected to that slot with a true value. finishThread should be provided in the loop condition to end it when it is set to true. After that wait for the thread to finish properly for some seconds and force it to terminate if it did not finish.

So you can have in your destructor :

emit setThreadFinished(true); //Tell the thread to finish
monitorThread->quit();
if(!monitorThread->wait(3000)) //Wait until it actually has terminated (max. 3 sec)
{
    monitorThread->terminate(); //Thread didn't exit in time, probably deadlocked, terminate it!
    monitorThread->wait(); //We have to wait again here!
}

How to safely terminate objects running on different threads in QT

You're doing some funky things.

1) Each QThread is having start() called twice - first in the testThread constructor, then again in main(). The second call won't do anything, since is is already running.

2) When you start the QThread in the testThread constructor, your loop will execute and emit the signals before they are connected - it makes me think you don't actually want to start the thread in the constructor, and instead leave it unstarted until later in main().

3) QThread::quit() will cause the thread's event loop to return - this is what you're missing. connect a "done" type signal from testThread to the quit slot, and then QThread::wait() will behave as you expect, with the calls returning once the loops are completed.

4) I know it's just a little test program, but you're leaking memory since you aren't calling delete on your new objects. You actually don't need to use new at all here, everything can be allocated on the stack - generally a better idea.

Improperly terminating a thread?

Generally speaking, terminating a running thread leads to undefined behavior (as opposed to a thread that's blocked in a state where other data is consistent). You shouldn't be terminating the threads. Simply execute the deleteLater on the subprocesses, and they'll dispose of themselves automatically. Their threads won't be affected. See this answer for why it's OK to call deleteLater on objects in other threads.

You should keep a pool of threads for reuse, no need to recreate them. You can even reuse the QThreadQueue for that.

A default QThread is very much unsafe to destroy, and you should use a version without that deficiency instead, if you can.

How to stop a Qthread after calling movetothread()?

First of all Qt has nice asynchronous API so use of threads (in this context) is totally obsolete and waste of resources.

By using lambda in connect statement you have lost thread hopping functionality, which is done by default by connect. Result is that you are trying to destroy thread object from thread which this object manages.

Note that QObject::connect last argument is: Qt::ConnectionType type = Qt::AutoConnection (connections without lambda). When lambda is used QObject::connect do not have such functionality and code is invoked from thread which has invoked signal.

Dropping lambda will ensure that each invoke is done on proper thread.

void TcpServer::incomingConnection(qintptr handle)
{
    QThread* tcpThread = new QThread;
    TcpSocket* tcpSocket = new TcpSocket;

    tcpSocket->setSocketDescriptor(handle);
    tcpSocket->moveToThread(tcpThread);

    connect(tcpSocket, &TcpSocket::sigRecvFinish, tcpSocket, &QBject::delteLater);
    connect(tcpSocket, &TcpSocket::sigRecvFinish, tcpThread, &QThread::quit);
    connect( tcpThread, &QThread::finished, tcpThread, &QThread::deleteLater );

    tcpThread->start();
}

For technical details see:

Qt Namespace | Qt Core 5.15.3

Constant	Value	Description
Qt::AutoConnection	0	(Default) If the receiver lives in the thread that emits the signal, Qt::DirectConnection is used. Otherwise, Qt::QueuedConnection is used. The connection type is determined when the signal is emitted.

How to properly stop QThread run() function?

Instead of while(true) check for a "should end" condition, e.g. isInterruptionRequested().

Then, in main(), before returning, you tell the thread to stop, e.g. with QThread::requestInterruption and then wait on the thread

view.show();

const int ret = app.exec();

producer.requestInterruption();
producer.wait();

return ret;

What happens to QThread when application is being closed without proper wait() call?

QThread has, basically, a long-standing API bug: it isn't always in a destructible state. In C++, an object is considered to be in destructible state when it's safe to invoke its destructor. Destructing a running QThread is an error. A QThread is merely a thread controller, it's not the "thread" itself. Think of how QFile acts: you can destruct it at any time, whether it's open or not. It truly encapsulates the notion of a file as a resource. A QThread is too thin of a wrapper around the native (system) thread: when you destruct it, it does not terminate nor dispose of the native thread if there is one. This is a resource leak (threads are OS resources), and people trip over this issue over and over again.

When the application's main() function returns, your implementation of the C/C++ runtime library happens to terminate all of the application's threads, effectively terminating the entirety of the application. Whether this is the behavior you desire is up to you. You're supposed to quit() and wait() your event-loop-running thread. For threads without an event loop, quit() is a no-op and you must implement your own quit flag. You must wait() on the thread before you destruct it. This is to prevent race conditions.

Below is a safe wrapper for QThread. It is a final class, since you can't reimplement run. This is important, since a reimplementation of run could be done in such a way that makes quit a no-op, breaking the contract of the class.

#include <QThread>
#include <QPointer>

class Thread : public QThread {
   using QThread::run; // final
public:
   Thread(QObject * parent = 0) : QThread(parent) {}
   ~Thread() { quit(); wait(); }
};

class ThreadQuitter {
public:
   typedef QList<QPointer<Thread>> List;
private:
   List m_threads;
   Q_DISABLE_COPY(ThreadQuitter)
public:
   ThreadQuitter() {}
   ThreadQuitter(const List & threads) : m_threads(threads) {}
   ThreadQuitter(List && threads) : m_threads(std::move(threads)) {}
   ThreadQuitter & operator<<(Thread* thread) { 
     m_threads << thread; return *this;
   }
   ThreadQuitter & operator<<(Thread& thread) {
     m_threads << &thread; return *this;
   }
   ~ThreadQuitter() {
      foreach(Thread* thread, m_threads) thread->quit();
   }
};

It could be used as follows:

#include <QCoreApplication>

int main(int argc, char ** argv) {
   QCoreApplication app(argc, argv);
   QObject worker1, worker2;
   Thread thread1, thread2;
   // Style 1
   ThreadQuitter quitter;
   quitter << thread1 << thread2;
   // Style 2
   ThreadQuitter quitterB(ThreadQuitter::List() << &thread1 << &thread2);
   //
   worker1.moveToThread(&thread1);
   worker2.moveToThread(&thread2);
   thread1.start();
   thread2.start();

   QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection);
   return app.exec();
}

Upon return from main, the thread quitter will quit() all worker threads. This allows the threads to wind down in parallel. Then, thread2.~Thread will wait for that thread to finish, then thread1.~Thread will do the same. The threads are now gone, the objects are threadless and can be safely destructed: worker2.~QObject is invoked first, followed by worker1.~QObject.

---

Related Topics

What Is the Purpose of a Declaration Like Int (X); or Int (X) = 10;

Adding Signals/Slots (Qobject) to Qgraphicsitem: Performance Hit

Why Isn't There Int128_T

Lifetime of Object Is Over Before Destructor Is Called

Ramifications of C++20 Requiring Two's Complement

What's the Difference Between Parentheses and Braces in C++ When Constructing Objects

Very Simple Application Fails with "Multiple Target Patterns" from Eclipse

How to Create a Std::Set of Structures

Generating Random Enums

Copy Constructor VS. Return Value Optimization

How to Get a Non-Const C String Back from a C++ String

How to Make an Image Resize to Scale in Qt

Equivalent of "Using Namespace X" for Scoped Enumerations

When Pass a Variable to a Function, Why the Function Only Gets a Duplicate of the Variable

How to Find the Current Directory

Boost Asio - How to Write Console Server

What Does Slicing Mean in C++

Why "Foo F(Bar());" Can Be a Declaration of a Function That Takes Type Bar and Returns Type Foo

---

Leave a reply

Submit