QObject::killTimers() warning after timer stopped from its own thread

[astodolski]

Hmmm.... whaaaat?

Please read the thread to help with confusion. I don't understand the quip. I was replying to someone else's comment.

[wysota]

I had read the thread before posting. It didn't help with my confusion

So is your problem fixed or not?

[amleto]

Ok. I made the illustration (with code) as to the point I was making - less the semantics. The change in use of the timer changes the outcome. I did this change from referencing the article which points to proper use of threads. The other prior suggestions didn't affect the outcome of the message warning.

Unfortunately your semantics (talking about references) are plain wrong and need to be corrected if you want to continue talking in c++ parlance.

re bolded sentence: Well it will, or it wont, or it might. It all depends on whether you correctly pass parent pointer to the qtimer and whether there is truly a Qt bug here - you still haven't clarified your change (with code or without) - less or including semantics.

[wysota]

Guys, but where do you see a Qt bug here?

[astodolski]

Guys, but where do you see a Qt bug here?

This comment looks like the right question. I don't know if it's a bug. I was working on the suggestions of things to try. I then added a change on my own. This then devolved into trivial banter of what to call a parameter passed to connect. I just want to get the issue resolved as there seems to be lots of discussion on threading in general. I posted the change in the call to connect with a code snippet. It shows a different result. Is it a bug? I don't know. Did I sin in misquoting c++ scripture? Perhaps. All who have read this knows what was intended and I'm just trying to understand. The code posted in total I hope is enough (short of a complete app) to answer the relevant question as to whether the thread was moved, and/or whether the parent pointer was passed to the timer. This is shown at top code listing. Whether wrong or a bug IS the discussion.

przejrzystość

[wysota]

In my opinion there is no bug here. If I'm right then sometimes you are not stopping the timer before the thread ends. Thus when at some point the object is destructed from within the main thread, the destructor of the timer calls stop() and since the object has a different thread affinity, you get the warning message. You either have to move the worker object back to the main thread before the worker thread ends or you have to make sure the timer is stopped before the thread ends. You'd need some "aboutToFinish" signal that you can use to stop the timer.

For instance this line of your code is surely invalid:

connect(workerThread, SIGNAL(finished()), worker, SLOT(stop()));

connect(workerThread, SIGNAL(finished()), worker, SLOT(stop()));

To copy to clipboard, switch view to plain text mode 

workerThread and worker live in different threads (wT in main thread, w in worker thread) thus the connection is going to be queued and effectively stop() will likely never be called because it'd require workerThread's event loop to be running (which it isn't because the thread has just finished() (after exiting QThread::run()).

I suggest to sit down with a piece of paper, a pencil and draw some kind of graph or list of states and transitions that need to happen during the lifetime of the thread and the worker. Pay special attention to thread affinity so that you are 100% sure when a particular slot is going to be called. Then implement your state machine. It might be easiest to just push the object back to the original thread before the worker thread dies, e.g. by doing the following:

void MyThread::run() {
  exec();
  if(worker)
    worker->moveToThread(QCoreApplication::instance()->thread());
}

void MyThread::run() {
  exec();
  if(worker)
    worker->moveToThread(QCoreApplication::instance()->thread());
}

To copy to clipboard, switch view to plain text mode 

Another possibility would be to connect to the finished() signal with Qt::DirectConnection to do some cleanup before the thread dies assuming that the signal is emitted in the context of the worker thread. If not, subclass QThread and make run() emit some signal (e.g. aboutToFinish()) before it returns.

[astodolski]

In my opinion there is no bug here. If I'm right then sometimes you are not stopping the timer before the thread ends.

That was related to a previous question I WAS going to attempt to answer

Thus when at some point the object is destructed from within the main thread, the destructor of the timer calls stop() and since the object has a different thread affinity, you get the warning message. You either have to move the worker object back to the main thread before the worker thread ends or you have to make sure the timer is stopped before the thread ends. You'd need some "aboutToFinish" signal that you can use to stop the timer.

aboutToFinish would be more convenient.

It might be easiest to just push the object back to the original thread before the worker thread dies, e.g. by doing the following:

void MyThread::run() {
  exec();
  if(worker)
    worker->moveToThread(QCoreApplication::instance()->thread());
}

void MyThread::run() {
  exec();
  if(worker)
    worker->moveToThread(QCoreApplication::instance()->thread());
}

To copy to clipboard, switch view to plain text mode 

This approach then suggests the use of subclassing QThread doesn't it?

Another possibility would be to connect to the finished() signal with Qt:irectConnection to do some cleanup before the thread dies assuming that the signal is emitted in the context of the worker thread. If not, subclass QThread and make run() emit some signal (e.g. aboutToFinish()) before it returns.

Another reason to continue use of subclass of QThread.
Thank you.

[wysota]

This approach then suggests the use of subclassing QThread doesn't it?

Yes. There is nothing wrong with subclassing QThread. It gets to be wrong when you start adding slots to the subclass

However you don't have to subclass QThread at all. You can emit the signal from the Worker object as well when it's done its work or just immediately move it to the main thread. It breaks some OOP concepts but it's still a valid approach.

[amleto]

ok, next try. Does this give same thread timer warning?

FlashWizard::FlashWizard(...)
{
...
  worker = new Worker;
 
  qDebug() << worker->thread();
 
  workerThread = new QThread;
  worker->moveToThread(workerThread);
 
  qDebug() << worker->thread();
  qDebug() << worker->m_workerTimer->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)));
 
  workerThread->start();
  QMetaObject::invokeMethod(worker, "start");
  QMetaObject::invokeMethod(worker, "stop");
}

FlashWizard::FlashWizard(...)
{
...
  worker = new Worker;

  qDebug() << worker->thread();

  workerThread = new QThread;
  worker->moveToThread(workerThread);

  qDebug() << worker->thread();
  qDebug() << worker->m_workerTimer->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)));

  workerThread->start();
  QMetaObject::invokeMethod(worker, "start");
  QMetaObject::invokeMethod(worker, "stop");
}

To copy to clipboard, switch view to plain text mode 

Yes the same message. <SNIP>

Guys, but where do you see a Qt bug here?

That is a bug if that truly happened. But it seems the OP is not being honest/accurate.

[wysota]

This works fine for me on Qt 4.8 and 5.0 RC2. I'm not getting any warnings.

include <QtCore>
 
class Worker : public QObject {
  Q_OBJECT
public:
  Worker() : QObject(){
    timer = new QTimer(this);
    timer->setInterval(1000);
  }
public slots:
  void start() { qDebug() << Q_FUNC_INFO; timer->start(); }
  void stop() { qDebug() << Q_FUNC_INFO; timer->stop(); }
private:
  QTimer *timer;
};
 
#include "main.moc"
 
int main(int argc, char **argv) {
  QCoreApplication app(argc, argv);
  QThread thread;
  Worker *worker = new Worker;
  worker->moveToThread(&thread);
  thread.start();
  QMetaObject::invokeMethod(worker, "start");
  QMetaObject::invokeMethod(worker, "stop");
  QObject::connect(&app, SIGNAL(aboutToQuit()), &thread, SLOT(quit()));
  QTimer t;
  t.start(1000);
  QObject::connect(&t, SIGNAL(timeout()), &app, SLOT(quit()));
  app.exec();
  thread.wait();
}

include <QtCore>

class Worker : public QObject {
  Q_OBJECT
public:
  Worker() : QObject(){
    timer = new QTimer(this);
    timer->setInterval(1000);
  }
public slots:
  void start() { qDebug() << Q_FUNC_INFO; timer->start(); }
  void stop() { qDebug() << Q_FUNC_INFO; timer->stop(); }
private:
  QTimer *timer;
};

#include "main.moc"

int main(int argc, char **argv) {
  QCoreApplication app(argc, argv);
  QThread thread;
  Worker *worker = new Worker;
  worker->moveToThread(&thread);
  thread.start();
  QMetaObject::invokeMethod(worker, "start");
  QMetaObject::invokeMethod(worker, "stop");
  QObject::connect(&app, SIGNAL(aboutToQuit()), &thread, SLOT(quit()));
  QTimer t;
  t.start(1000);
  QObject::connect(&t, SIGNAL(timeout()), &app, SLOT(quit()));
  app.exec();
  thread.wait();
}

To copy to clipboard, switch view to plain text mode 

You can build on this example to check your actual code (with timers and stuff).

Edit: I even added the timer stuff to the code, still works as expected.

[astodolski]

I've compiled a complete application with the changes that have been posted in this thread. It's possible as you have suggested that the app is attempting to stop the timer before the thread completes.

test app attached here ->> workerTest.zip

[wysota]

Does the code you posted still contain the bug? I'm not getting any warnings.

[astodolski]

Does the code you posted still contain the bug? I'm not getting any warnings.

The code example I posted still reports the warning message. Are you able to compile and run it? I am using 4.8.4 with Win SDK 7.1 compiler under Creator 2.6

[wysota]

I'm not getting any warnings. I use Qt 4.8.3 x64 Linux. Same output with Qt 5.0 RC2.

$ ./FlashWizard
Main thread: QThread(0xcde710)
Worker Object Thread Id: QThread(0x7ffff75eff00)
Now using thread: QThread(0x7ffff75eff00)
1
2
3
...
97
98
99
100

[astodolski]

I'm not getting any warnings. I use Qt 4.8.3 x64 Linux. Same output with Qt 5.0 RC2.

Oh I guess the problem wasn't fully realized. It's only when you click the cancel button before the loop finishes that the message is shown - sorry. I should have re-iterated that. I though that was evident within the thread.

[wysota]

Still, no warning

An idea: maybe you shouldn't be using a thread at all? What do you need it for?

[astodolski]

Still, no warning

An idea: maybe you shouldn't be using a thread at all? What do you need it for?

So let me understand. You run this app. Before the count gets to 100, hit the brakes with the Cancel button. You're telling me no message like "QObject::killTimer: timers cannot be stopped from another thread"?

This is a stripped out app that is used as a prototype for use with a hardware DLL. That library has functions that are called for each timer tick and have to perform tasks in the background that would otherwise tie up the UI. I created this to perform a task (a loop counter in this case) and update a progress bar. I find it incredible that this test app loaded on a host PC and on a virtual machine and on my own laptop all perform exactly the same - that is, the killTimer warning is displayed in the debug window after the Cancel button is clicked and before the counter gets to 100, except for you.

I read your article on keeping the UI responsive as well as general considerations when making calls to hardware while updating a UI with background workers. I feel that background tasks belong in another thread so as to keep the UI responsive. That's my reason for this prototype. That's pretty much boilerplate practice. I don't have a Linux distro prepared for testing on there.