Using QThread and moveToThread properly with QTimer and QTcpSocket

Question

From reading this blog, this blog and some others, Subclassing QThread is bad practice. So I tried to apply this method.

But my problem is that I have a QTimer and a QTcpSocket in the class I want to move to a different thread. Suddenly, it's not as easy as it the examples used. :(

QThread m_commsThread;
m_pICommsHandler = new CommsHandlerIP();
m_pICommsHandler->moveToThread(&m_commsThread);
m_commsThread.start();

And here is the CommsHandlerIP class, methods are not included.

class CommsHandlerIP : public QObject
{
    Q_OBJECT
public:
    CommsHandlerIP();
    ~CommsHandlerIP(void);
protected:
    QTcpSocket m_TCPSocket;
    QTimer m_timer;
}

Issue is that the QTimer and the QTcpSocket (inside CommsHandlerIP class) are in the main thread even if you move CommsHandlerIP. So I can't start the timer or connect the socket.

If I try to moveToThread the QTimer and QTcpSocket (inside the constructor by passing the thread pointer for instance), this become really messy when I leave the app.

What should I do?

Hath

Answer 1

Class Instances are created on the calling thread. QTimer inherits QObject. Each Thread on Qt can have an event loop if it calls exec(). so you want to move QTimer to an event loop on another thread. so you should manually move it.

Therefore, delay their creation until after you move the object: -

class CommsHandlerIP : public QObject
{
    Q_OBJECT

    public slots:
       void Initialise();

    private: 
       void Run();

       // c++ 11, initialising in headers...
       QTimer* m_pTimer = NULL;
       QTcpSocket* m_pSocket = NULL;   
};

void CommsHandlerIP::Initialise()
{
     m_pTimer = new QTimer(this);
     m_pSocket = new QTcpSocket(this);

     Run();
}

QThread m_commsThread;
m_pICommsHandler = new CommsHandlerIP();

// Note Qt 5 connect style
connect(&m_commsThread, &QThread::started, m_pICommsHandler, &CommsHandlerIP::Initialise);
m_pICommsHandler->moveToThread(&m_commsThread);
m_commsThread.start();

When the thread is started, the CommsHanderIP Initialise function is called; this is where you should create and setup the QTcpSocket and QTimer objects before calling Run(). As the CommsHandlerIP is running in the new thread before creating those objects, they will also share the same thread affinity.

Stansbury

I feel so stupid, I knew what was the problem but I tried to fix it in a compltely wrong way – Hath

It's easy when you know how ;O) – Stansbury

If you've passed a parent to the QTcpSocket, it will be deleted along with its parent; such is the power of parent / child hierarchy in Qt. – Stansbury

Passing this as a parent to m_TCPSocket and m_timer when creating them in the constructor would be sufficent. This is because QObject::moveToThread() changes the thread affinity for this object and its children. So you don't have to delay their creation. – Proximal

True, but problems can occur later, if connections to signals / slots have been made to them before they have been moved. The connection type created depends upon the thread affinity of the caller and callee. – Stansbury

Actually it ins't enough. If I close the main thread (and thus closing the worker thread), then for some reasons, it won't work. (can't send event to the socket from a different thread, even though this is in the destructor of the CommsHandler). – Hath

Define "won't work". You should be stopping the other threads (call quit) and waiting for them to finish before terminating the application. – Stansbury

in the main thread (the QMainWindow), i have m_commsThread.quit() and m_commsThread.wait() and then delete m_pICommsHandler; (object moved to the other thread) I managed to fix the issue by calling the destruction of the socket just before the main thread is exited. – Hath

@Merlin069 AFAIK, if the type is Qt::AutoConnection, it shouldn't matter if the connection is established before moveToThread() is called. This is because If the signal is emitted from a different thread than the receiving object, the signal is queued, behaving as Qt::QueuedConnection. Otherwise, the slot is invoked directly, behaving as Qt::DirectConnection. The type of connection is determined when the signal is emitted. – Proximal

Thuga is right. By using this, I don't need to delay the initialisation of the socket and of the timers. The destructor issue remains in this case though. – Hath

@thuga, just checked and you're right, the docs state "The type of connection is determined when the signal is emitted." Thanks for pointing that out. – Stansbury

Answer 2

I stumbled across this when searching on Timer behaviour and movetoThread. The accepted answer is a good work-around but not really the root cause of the problem. There is a general rule that when you move an object then all child objects will move as well. So you just need to make sure that the QTimer becomes a child so pass the this pointer in its constructor.

CommsHandlerIPL::CommsHandlerIP()
: QObject(), m_pTimer(new QTimer(this))        // <=== crucial to make it a "child" object 
{
}

Monofilament

Answer 3

There is a much simpler method of achieving all this that follows the same algorithm but doesn't involve all the boilerplating needed to create threads and changing thread affinities, using QRunnable and QThreadPool

If I convert Merlin069's example, you'll see how it simplifies the code a bit:

class CommsHandlerIP : public QObject, public QRunnable
{
    Q_OBJECT
    public:
       void run();

    public slots:
        //... any slots

    signals:
        //... any signals

    private:
       // c++ 11, initialising in headers...
       QTimer* m_pTimer = NULL;
       QTcpSocket* m_pSocket = NULL;   
};

void CommsHandlerIP::run()
{
     m_pTimer = new QTimer();
     m_pSocket = new QTcpSocket();

     //...

     delete m_pTimer;
     delete m_pSocket;
}

QThreadPool::globalInstance()->start(new CommsHandlerIP);

Overturn