Correctly stop QTimer after its QThread has already quit?

TheLoneMilkMan

If an object is moved to a new thread, all of its child objects are also moved to that thread. If a QTimer is a child it restarts its timer in the new event loop. I recently started having problems when allocating objects with automatic storage duration instead of with new and deleteLater.

The problem ends up being that a make an instance of a class with a child QTimer that is then moved to a new thread. Then when the first object is destroyed, the QThread is quit and then destroyed. However, this doesn't stop the timer and it seems to be left in sort of a limbo state. When the QTimer destructor is called, it's called in the original thread which Qt doesn't like since it's still running, and I get these messages:

QObject::killTimer: Timers cannot be stopped from another thread
QObject::~QObject: Timers cannot be stopped from another thread

I try to use invokeMethod to stop the timer from the correct thread but the method is never called since the thread has been quit. There also doesn't seem to be a way to get the timer back to the current thread as moveToThread can only be used to "push", but the thread to push from is gone. So I'm wondering if there is a way to stop a QTimer if the thread where its event loop is running has been stopped?

main.cpp:

#include <QApplication>

#include "mainwindow.h"

int main(int argc, char* argv[])
{
    QApplication a {argc, argv};
    MainWindow w;
    w.show();

    return a.exec();
}

mainwindow.h:

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <QThread>

#include "ui_mainwindow.h"
#include "testclass.h"

class MainWindow : public QMainWindow
{
    Q_OBJECT
public:
    explicit MainWindow(QWidget* parent = nullptr);
    ~MainWindow();

private:
    Ui::MainWindow m_ui;
    TestClass m_test;
    QThread m_thread;
};

#endif // MAINWINDOW_H

mainwindow.cpp:

#include "mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow{parent}
{
    m_ui.setupUi(this);
    m_test.moveToThread(&m_thread);
    m_thread.start();
}

MainWindow::~MainWindow()
{
    m_thread.quit();
    m_thread.wait(500);
}

testclass.h:

#ifndef TESTCLASS_H
#define TESTCLASS_H

#include <QObject>
#include <QTimer>

class TestClass : public QObject
{
    Q_OBJECT
public:
    TestClass();
    ~TestClass();

private:
    QTimer m_timer;
};

#endif // TESTCLASS_H

testclass.cpp

#include "testclass.h"

#include <QMetaObject>

TestClass::TestClass() :
    m_timer{this}
{
    m_timer.start();
}

TestClass::~TestClass()
{
    QMetaObject::invokeMethod(&m_timer, "stop");   // Not called because thread has quit
}

SGaist

Hi,

Why use invokeMethod since m_timer belongs to TestClass ?

Also, depending on what you want to do with your timer, QObject::startTimer might be a better choice.

TheLoneMilkMan

In the current case I'm using invokeMethod because just calling the method causes stop to be called in the first thread and the same error messages appear. Using invokeMethod the call is just ignored. I'm not sure if it's actually dangerous though as the thread is stopped. I'll have to look into QObject::startTimer though and see if it behaves any better.

SGaist

Usually, objects that you expect to move are allocated on the heap and not on the stack. You might even trigger a double delete.

TheLoneMilkMan

The problem is that the moved object needs to be deleted before another object. If I do

QThread thread;
auto obj1 = new Class1;
auto obj2 = new Class2(*obj1);  // Must delete before obj1 or else the reference is invalid
connect(thread, &QThread::finished, obj2, &Class2::deleteLater);
connect(thread, &QThread::finished, obj1, &Class1::deleteLater);
obj2->moveToThread(&thread);
thread.start();

Are the two objects guaranteed to be deleted in the right order?

SGaist

The first thing that is fishy is that you dereference obj1. QObject based classes cannot be copied.

What I was suggesting is just to create your QTimer member on the heap in your TestClass` constructor.

kshegunov

@TheLoneMilkMan said in Correctly stop QTimer after its QThread has already quit?:

So I'm wondering if there is a way to stop a QTimer if the thread where its event loop is running has been stopped?

You have to create the root of the object tree, the one you will be moving to a separate thread (i.e. the TestClass instance) in the heap. You can't use the stack here, as it will unwind after the thread has stopped, hence you get the mentioned errors. You can keep the timer with auto-storage, only the root object needs to be allocated with new (and of course to connect QThread::finished to TestClass::deleteLater).

SGaist

@kshegunov, AFAIK, every QObject member of the class to be moved should be properly parented, otherwise they won't be moved to the new thread.

kshegunov

Indeed, that is true, and it is the case for the example code.

Dale M

In TestClass, add a method:

void moveToThread(QThread *thread)
{
connect(thread, &QThread::finished, &m_timer, &QTimer::stop);
QObject::moveToThread(thread);
}

This will make sure when the thread the object is moved to terminates, the timer gets stopped by that thread.