QThread and timers

JKSH

Please share your code that shows:

Where is your QTimer constructed?

Where is your QTimer destroyed?

Remember, a QThread object is not a thread: It lives in the main thread -- it doesn't live in the other thread. Which means:

An object that is created in the QThread's constructor will live in the main thread

An object that is destroyed with the QThread is destroyed from the main thread

astodolski

I would like to post a complete test app which illustrates the condition. The error is when the stop() slot is called. It appears to be called twice but that's another discussion. On the first call of the stop slot, killTimer calls unregisterTimer which gets to line 957 of qeventdispatcher_win.cpp. (Windows only?) This is where @thread() != QThread::currentThread;@

on a second call the current thread equals the worker thread. I'm not seeing how to post a sample attachment. Is this allowed?

Peter K

Make sure you stop the timer in the thread it lives in.

Is the timer a member of a derived QObject or a derived QThread? Note that the QThread object itself lives in the main thread, so it's timers should be deleted in the main thread.

JKSH

There's no way to attach files at this time, but if you can make a minimal test case, feel free to paste the code straight into your reply (just use the '@' tags like you did in your last post)

astodolski

OK, Let's see how well this illustrates what I have going on:

worker.h

@
class Worker : public QObject
{
Q_OBJECT
public:
explicit Worker(QObject *parent = 0);
virtual ~Worker();

signals:
void finished();
void updateProgress(int);

public slots:
void start();
void stop();
void doWork();

private:
QTimer* m_workerTimer;
int m_tid;
void timerEvent(QTimerEvent *ev);
};
@

worker.cpp

@
#include "worker.h"
#include <QThread>
#include <QDebug>

volatile bool stopWork;

Worker::Worker(QObject *parent) : QObject(parent)
{
stopWork = false;
}

Worker::~Worker()
{

}

void Worker::start()
{
m_tid = startTimer(100);
}

void Worker::stop()
{
killTimer(m_tid);
stopWork = true;
}

void Worker::doWork()
{
for (int i = 0; i < 1000000; i++)
{
if (stopWork)
{
break;
}

    if (i % 10000 == 0)
    {
        emit updateProgress((i / 10000) + 1);
        qDebug() << ((i / 10000) + 1);
        static_cast<MThread*>(QThread::currentThread())->msleep(20);
    }
}

emit finished();

}

void Worker::timerEvent(QTimerEvent* ev)
{
if(ev->timerId() == m_tid)
doWork();
else
QObject::timerEvent(ev);
}
@

flashwizard.h

@
#ifndef FLASHWIZARD_H
#define FLASHWIZARD_H

#include <QWizard>
#include <QtCore>
#include <QThread>
#include "worker.h"

namespace Ui
{
class FlashWizard;
}

class FlashWizard : public QWizard
{
Q_OBJECT

public:
explicit FlashWizard(QWidget* parent = 0);
virtual ~FlashWizard();

enum { Page_Intro, Page_Program};

public slots:
void updateWritingProgressBar(const int value);
void updateLabel(const QString& value);
void pageCleanUp(int id);
void reject();

private:
Ui::FlashWizard* ui;
bool convertedOK;
Worker* worker;
QThread workerThread;

};

#endif // FLASHWIZARD_H

@

flashwizard.cpp

@
#include "flashwizard.h"
#include "ui_flashwizard.h"
#include "worker.h"

#include <QThread>
#include <QDebug>
#include <QMessageBox>

FlashWizard::FlashWizard(QWidget *parent) : QWizard(parent), ui(new Ui::FlashWizard)
{
ui->setupUi(this);

ui->pbReading->reset();
ui->pbWriting->reset();

ui->lblReadingStatus->setText("");
ui->lblReadingStatus->setText("");
ui->lblProgStatus->setText("");

connect(ui->introPage, SIGNAL(p1_updateLabelOne(const QString&)), this, SLOT(updateLabel(const QString&)));
connect(this, SIGNAL(currentIdChanged(int)), this, SLOT(pageCleanUp(int)));

worker = new Worker;
worker->moveToThread(&workerThread);

qDebug() << "Worker Object Thread Id: "<< worker->thread();

connect(&workerThread, SIGNAL(started()), worker, SLOT(start()));
connect(&workerThread, SIGNAL(finished()), worker, SLOT(stop()));
connect(worker, SIGNAL(finished()), &workerThread, SLOT(quit()));
connect(worker, SIGNAL(finished()), ui->programPage, SLOT(setDone()));
connect(worker, SIGNAL(updateProgress(int)), this, SLOT(updateWritingProgressBar(int)));

}

FlashWizard::~FlashWizard()
{
delete ui;
}

void FlashWizard::updateWritingProgressBar(const int value)
{
if (ui->pbWriting->maximum() == value)
ui->pbWriting->hide();
ui->pbWriting->setValue(value);
}

void FlashWizard::updateLabel(const QString &value)
{
ui->lblReadingStatus->setText(value);
}

void FlashWizard::pageCleanUp(int id)
{
if (id == Page_Program)
{
ui->lblProgStatus->setText("Updating ScanX");
workerThread.start();
qDebug() << "Now using thread: " << worker->thread();
}

ui->pbReading->setVisible(false);
ui->lblUpdating->setText("");

}

void FlashWizard::reject()
{
worker->stop();
}

@

It's a wizard that displays a progress bar in the second page and when the cancel button is clicked, the worker should stop - which it does but I get the killTimer message.

JKSH

[quote author="astodolski" date="1356056195"]
@
void FlashWizard::reject()
{
worker->stop();
}@
[/quote]That's a direct function call. Your main thread executes FlashWizard::reject(), thus it will also execute Worker::stop(), thus it will also execute Worker::killTimer(). In other words, the program is attempting to kill the timer from the main thread.

To execute Worker::stop() in your other thread, connect the your button's clicked() signal to Worker::stop(). You can get rid of FlashWizard::reject() altogether.

Edit: To clarify, a slot is just a regular C++ function. It is technically possible to call a slot from any thread, but we usually don't want this. To make the call safely, we use a queued signal-slot connection -- Qt notifies the event loop of the "correct" thread when the signal is emitted, and the slot is then called from that thread

Looks like there is a bug in Qt though, if the Linux implementation fails to warn the developer about the cross-thread timer manipulation!

lgeyer

It should indeed warn on Linux as well. Did you define QT_NO_DEBUG to suppress the warning?

astodolski

[quote author="JKSH" date="1356061869"][quote author="astodolski" date="1356056195"]
@
void FlashWizard::reject()
{
worker->stop();
}@
[/quote]That's a direct function call. Your main thread executes FlashWizard::reject(), thus it will also execute Worker::stop(), thus it will also execute Worker::killTimer(). In other words, the program is attempting to kill the timer from the main thread.

To execute Worker::stop() in your other thread, connect the your button's clicked() signal to Worker::stop(). You can get rid of FlashWizard::reject() altogether.

Edit: To clarify, a slot is just a regular C++ function. It is technically possible to call a slot from any thread, but we usually don't want this. To make the call safely, we use a queued signal-slot connection -- Qt notifies the event loop of the "correct" thread when the signal is emitted, and the slot is then called from that thread

Looks like there is a bug in Qt though, if the Linux implementation fails to warn the developer about the cross-thread timer manipulation![/quote]

I tried doing it this way:

@connect(button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));@

I get an error:

C2664: 'bool QObject::connect(const QObject *,const char *,const QObject *,const char *,Qt::ConnectionType)' : cannot convert parameter 1 from 'QAbstractButton *' to 'const QObject *'
Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

I casted the button to QObject* and the slot does not get executed when I click the Wizard cancel button.

astodolski

[quote author="Lukas Geyer" date="1356075094"]It should indeed warn on Linux as well. Did you define QT_NO_DEBUG to suppress the warning?[/quote]

No I didn't try that - good to know. I prefer to understand and prevent the warning. Thanks for the tip.

Could be a Qt bug.

JKSH

[quote author="astodolski" date="1356188428"]I tried doing it this way:

@connect(button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));@

I get an error:

C2664: 'bool QObject::connect(const QObject *,const char *,const QObject *,const char *,Qt::ConnectionType)' : cannot convert parameter 1 from 'QAbstractButton *' to 'const QObject *'
Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

I casted the button to QObject* and the slot does not get executed when I click the Wizard cancel button.[/quote]Hmm... QAbstractButton inherits QWidget which inherits QObject, so it should work. Perhaps QAbstractButton was simply forward-declared by the QWizard header. Try adding #include <QAbstractButton>

lgeyer

[quote author="astodolski" date="1356188639"][quote author="Lukas Geyer" date="1356075094"]It should indeed warn on Linux as well. Did you define QT_NO_DEBUG to suppress the warning?[/quote]
No I didn't try that - good to know. I prefer to understand and prevent the warning. Thanks for the tip.[/quote]

Please do not misunderstand me: I do not recommend using QT_NO_DEBUG to supress the warning. I was just curious why you didn't get the warning on your embedded device, and one reason might be that you have QT_NO_DEBUG defined.

astodolski

[quote author="Lukas Geyer" date="1356255159"][quote author="astodolski" date="1356188639"][quote author="Lukas Geyer" date="1356075094"]It should indeed warn on Linux as well. Did you define QT_NO_DEBUG to suppress the warning?[/quote]
No I didn't try that - good to know. I prefer to understand and prevent the warning. Thanks for the tip.[/quote]

Please do not misunderstand me: I do not recommend using QT_NO_DEBUG to supress the warning. I was just curious why you didn't get the warning on your embedded device, and one reason might be that you have QT_NO_DEBUG defined.[/quote]

I don't target an embedded device - I'm targeting Desktop.

astodolski

[quote author="JKSH" date="1356229131"][quote author="astodolski" date="1356188428"]I tried doing it this way:

@connect(button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));@

I get an error:

C2664: 'bool QObject::connect(const QObject *,const char *,const QObject *,const char *,Qt::ConnectionType)' : cannot convert parameter 1 from 'QAbstractButton *' to 'const QObject *'
Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

I casted the button to QObject* and the slot does not get executed when I click the Wizard cancel button.[/quote]Hmm... QAbstractButton inherits QWidget which inherits QObject, so it should work. Perhaps QAbstractButton was simply forward-declared by the QWizard header. Try adding #include <QAbstractButton>[/quote]

@
connect((QObject*)button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));
@

The Cancel button has no effect on the process. I also included:

@#include <QAbstractButton>@

goli

I have embedded device, and I don't get this warrning on the device.
This is my code:
@
.h file:

#include <QObject>
#include <QThread>

class ThreadManager: public QObject
{
Q_OBJECT

public:
explicit ThreadManager();
virtual ~ThreadManager();

inline void startTheThread()
{
    m_thread->start();
}

protected:
virtual void run() = 0;

private slots:
void exec();

private:
QThread* m_thread;
};

.cpp file:

ThreadManager::ThreadManager():
QObject()
{
m_thread = new QThread();
moveToThread(m_thread);
connect(m_thread, SIGNAL(started()), this, SLOT(exec()));
connect(m_thread, SIGNAL(finished()), m_thread, SLOT(deleteLater()));
}

ThreadManager::~ThreadManager()
{
m_thread->quit();
}

void ThreadManager::exec()
{
run();
}
@

I create a class that inherit ThreadManager and in the run() function I create a timer with timerEvent:
@
startTimer(1000);
@

when the program end I get this warrning...

JKSH

[quote author="astodolski" date="1356287500"]
@
connect((QObject*)button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));
@

The Cancel button has no effect on the process.[/quote]I notice you have a custom Qt Designer UI for your FlashWizard. Are you still using the QWizard's built-in Cancel button, or did you implement a new one in Qt Designer? button(QWizard::CancelButton) returns a pointer to the built-in button, but this button doesn't exist if you implemented your own UI.

Also, the (QObject*) typecast is unnecessary -- the compiler should cast it for you automatically. Furthermore, if you ever need to make an explicit typecast, using qobject_cast<> is safer, as Qt will perform a run-time check to see if the cast is valid.

[quote author="goli" date="1356332295"]I create a class that inherit ThreadManager and in the run() function I create a timer with timerEvent:
@
startTimer(1000);
@

when the program end I get this warrning...[/quote]QThread IS a thread manager (see http://qt-project.org/doc/qt-5.0/qtcore/qthread.html for a full description, and a small example on how to use QThread). Your code is wrapping a thread manager in a thread manager, so I recommend redesigning it.

Read astodolski's code from 21 December and my reply (http://qt-project.org/forums/viewthread/22916/#107177 ) -- is your issue the same?

goli

JKSH , Thank you for your reply.
I don't have the same issue as astodolski. I create the timer in the run() function of the thread(even if I do it without thread manager).
I do it this way to create a event loop for the thread that is saperate from the main thread(GUI thread).
when I try to get the "finished()":http://qt-project.org/doc/qt-5.0/qtcore/qthread.html#finished signal and then kill the timer, I get this warrning, but not all the time...sometimes it appear and sometimes not.

JKSH

You're welcome, goli.

Some questions:

How do you kill the timer? Which thread does the killing?

It sounds like you're using a QObject's built-in timer -- which thread does that QObject live in?

goli

The Timer was created in the run(in the exec of the thread) function of the new thread with startTimer function. This way I create Event loop for this thread that is saperate from the main thread. I keep the ID of this timer and kill him when the thread is finished. The main thread is the one who kill the timer, but I can't see other way to kill this timer that was created in the exec of the thread...

JKSH

[quote author="goli" date="1356439206"]The Timer was created in the run(in the exec of the thread) function of the new thread with startTimer function. This way I create Event loop for this thread that is saperate from the main thread. I keep the ID of this timer and kill him when the thread is finished. The main thread is the one who kill the timer, but I can't see other way to kill this timer that was created in the exec of the thread...[/quote]Let's see if I understood your method: You subclassed a QThread (let's call it a SubclassedQThread), started a timer in SubclassedQThread::run(), and started an event loop by calling exec() In SubclassedQThread::run()?

Firstly, you must remember that QThread is NOT a thread; QThread is a thread manager. Code inside SubclassedQThread::run() runs in the other thread, BUT the SubclassedQThread object lives in the main thread.

So, your problem doesn't begin when you kill your timer; it begins when you START your timer, because:

Your SubclassedQThread lives in the main thread, but
You start the timer from the other thread, and

Now, timers are supposed to run in the same thread as the object they belong to, so your timer is started in the "wrong" thread.

Is this confusing? Don't worry, you are not alone. Many people have faced similar problems like yours, because of QThread's design.

QThread's developer realized that the design is unintuitive, so he made a new recommendation: Avoid subclassing QThread. This is especially important if you use an event loop in your new thread!

If you'd like, I can write an example to show you how to use timers in a multithreaded program properly. Before that though, I'd like to ask: What do you want your program to do? Why do you want a timer in a separate thread?

astodolski

[quote author="JKSH" date="1356353967"][quote author="astodolski" date="1356287500"]
@
connect((QObject*)button(QWizard::CancelButton), SIGNAL(clicked()), worker, SLOT(stop()));
@

The Cancel button has no effect on the process.[/quote]I notice you have a custom Qt Designer UI for your FlashWizard. Are you still using the QWizard's built-in Cancel button, or did you implement a new one in Qt Designer? button(QWizard::CancelButton) returns a pointer to the built-in button, but this button doesn't exist if you implemented your own UI.

Also, the (QObject*) typecast is unnecessary -- the compiler should cast it for you automatically. Furthermore, if you ever need to make an explicit typecast, using qobject_cast<> is safer, as Qt will perform a run-time check to see if the cast is valid.

[quote author="goli" date="1356332295"]I create a class that inherit ThreadManager and in the run() function I create a timer with timerEvent:
@
startTimer(1000);
@

when the program end I get this warrning...[/quote]QThread IS a thread manager (see http://qt-project.org/doc/qt-5.0/qtcore/qthread.html for a full description, and a small example on how to use QThread). Your code is wrapping a thread manager in a thread manager, so I recommend redesigning it.

Read astodolski's code from 21 December and my reply (http://qt-project.org/forums/viewthread/22916/#107177 ) -- is your issue the same?
[/quote]

I use the Cancel button from the Wizard created in the ui in the designer. That's why I originally used the reject() method.

QThread and timers

Where is your QTimer constructed?

Where is your QTimer destroyed?

An object that is created in the QThread's constructor will live in the main thread

An object that is destroyed with the QThread is destroyed from the main thread

I have embedded device, and I don't get this warrning on the device. This is my code: @ .h file:

.cpp file:

How do you kill the timer? Which thread does the killing?

It sounds like you're using a QObject's built-in timer -- which thread does that QObject live in?

I have embedded device, and I don't get this warrning on the device.
This is my code:
@
.h file: