Trouble while destroying thread

[Ocsidot]

Hello everyone,

I'm not really used to using Qt, but for my work, I'v to do with it :D. I 've tried to make a generic thread Worker for my actions in multithreaded application. Here is my code

ThreadWorker.h


#ifndef _THREADWORKER_H_
#define _THREADWORKER_H_

#include "QtCore/qobject.h"
#include "QtCore/qthread.h"
#include "QtCore/qtimer.h"

class ThreadWorker : public QObject
{
Q_OBJECT

private:
int _frequency;
QTimer _timer;

public:
ThreadWorker(int frequency = 300);
~ThreadWorker();

/*!
\brief Create a worker which call the specified slot in th specified thread
\param thread The thread to use
\param source The object which contains the slot to use
\param slot The slot to use
*/
void createWorker(QThread *thread, const QObject* source, const char *slot);

/*!
\brief Sets the call frequency for the threaded method
\param frequency Frequency in millsecond (default value: 300ms)
*/
void setFrequency(int frequency);


private slots:
void start();
void stop();

public slots:
/*!
\brief a slot to indicates the end of our current work
*/
void terminate();

signals:
void finished();
};

#endif


ThreadWorker.cpp


#include "ThreadWorker.h"

ThreadWorker::ThreadWorker(int frequency): _frequency(frequency), _timer(this)
{}

ThreadWorker::~ThreadWorker()
{
terminate();
}

void ThreadWorker::createWorker(QThread *thread, const QObject* source, const char *slot)
{
connect(&_timer, SIGNAL(timeout()), source, slot, Qt::DirectConnection);

connect(thread, SIGNAL(started()), this, SLOT(start()), Qt::DirectConnection);
connect(this, SIGNAL(finished()), thread, SLOT(quit()), Qt::DirectConnection);
connect(this, SIGNAL(finished()), this, SLOT(stop()), Qt::DirectConnection);

moveToThread(thread);
}

void ThreadWorker::setFrequency(int frequency)
{
_frequency = frequency;
}

void ThreadWorker::start()
{
_timer.setInterval(_frequency);
_timer.start();
}

void ThreadWorker::stop()
{
if(_timer.isActive())
{
_timer.stop();
}
}

void ThreadWorker::terminate()
{
if(_timer.isActive())
{
emit finished();
}
}


And here is the code which use the ThreadWorker


#include "ThreadWorker.h"

class MyObject : public QObject
{

Q_OBJECT

private
QThread _thread;
ThreadWorker _worker;

public:
MyObject();
~MyObject();
void start();
void stop();

public slots:
void process();
};

MyObject::MyObject(): _thread(this)
{}

MyObject::~MyObject()
{
if(_thread.isRunning())
{
_worker.terminate();
_thread.wait(500);
}
}

void MyObject::start()
{
if(_thread.isRunning())
{
return;
}

_worker.createWorker(&_thread, this, SLOT(process()));

_thread.start();
}

void MyObject::process()
{
//do something
}

void MyObject::stop()
{
_worker.terminate();
}


I run this code in unit tests with QTest library and I use the macro QTEST_MAIN.
The execution seems fine until the following error:

Program: …
Module: 4.7.4
File: global\qglobal.cpp
Line:

ASSERT failure in QCoreApplication::sendEvent: “Cannot send events to objects owned by a different thread. Current thread 3f6a90.
Receiver ‘’ (of type ‘ThreadWorker’) was created in thread 12ef5c”, file kernel\qcoreapplication.cpp, line...

I do not really see where it comes from :/. Only thing is that this does not occur if I do not put this line moveToThread (thread), however, in this case, the slot process() is never called.

Does anyone have any idea what I am doing wrong?

Thank you in advance for any help you can provide me

ps: Sorry if my english is not correct :/

[yeye_olive]

If I read your code correctly, something is wrong in the logic of the program: the MyObject instance does not live in the new thread but you try to force its slots to be executed there by a direct connection. You must not do that. The MyObject instance must live in the worker thread. If what you want to achieve is to execute MyObject::process() periodically in a worker thread, just move a MyObject and a QTimer there.

[Ocsidot]

Thank you for your response Yeye_olive. I've tried to do this in the creatWorker() method:



moveToThread(thread);
source->moveToThread(thread);


But the error is still the same :/

[amleto]

did you get rid of all the DirectConnection specifiers yet?

[yeye_olive]

By doing source->moveToThread(thread) you also move source's children, including the QThread, which is a common mistake. I suggest you rethink a bit the architecture of your solution. Here is a rough suggestion:


class Manager {
public:
Manager();
~Manager();
void add(QObject *o, const char *method);
private:
QThread thread;
QTimer timer;
};

Manager::Manager() {
timer.setInterval(42000);
QObject::connect(&thread, SIGNAL(started()), &timer, SLOT(start()));
timer.moveToThread(&thread);
thread.start();
}

Manager::~Manager() {
thread.quit();
thread.wait();
}

void Manager::add(QObject *o, const char *method) {
QObject::connect(&timer, SIGNAL(timeout), o, method);
o->moveToThread(&thread);
}

(Note: this code is incomplete, I have not tested it, not even tried to compile it. This is only to give a simple idea of what I would try. Note that I did not put anything to stop the endless calls to o->method() and push o back to the main thread. That requires a little more code.)

[Viper666]

I have small question what moveToThread do? and is some way how working with QWidget in my thread ?

[amleto]

move to thread essentially just changes a thread ID (i believe). Events and queued slot connections only get handled in the thread that they belong to. This is taken care of by the event loop in that thread (if it exists).

Since widget events can only be handled in the main thread, you will not have any success moving QWidgets to different threads, only QObjects are suitable.

Something that is a bit confusing:


void xxx::some_func_in_maina_thread()
{

QThread* thrd = new QThread;

QObject* obj = new QObject;


// thrd and obj are in main thread. thrd *represents* the thread, but thrd is actually a QObject in the main thread

obj->moveToThread(thrd); // changes the thread id of `obj` to the thread id of the thread that `thrd` represents
}

[Ocsidot]

Thanks to all of tou, I've find a solution. As Yeye_olive suggested, I've done this:



#ifndef _THREADWORKER_H_
#define _THREADWORKER_H_

#include "QtCore/qobject.h"
#include "QtCore/qthread.h"
#include "QtCore/qtimer.h"

class ThreadWorker : public QObject
{
Q_OBJECT

private:
int _frequency;
QTimer _timer;
QThread _thread;

public:
ThreadWorker(int frequency = 300);
~ThreadWorker();

/*!
\brief Create a worker which call the specified slot in th specified thread
\param thread The thread to use
\param source The object which contains the slot to use
\param slot The slot to use
*/
void createWorker(const QObject* source, const char *slot);

/*!
\brief Defines what to do when the thread ends
\param source The object which contains the slot to use
\param slot The slot to use
*/
void onWorkerEnded(const QObject* source, const char *slot);

/*!
\brief Sets the call frequency for the threaded method
\param frequency Frequency in millsecond (default value: 300ms)
*/
void setFrequency(int frequency);

/*!
\brief Starts the thread worker
*/
void start();

/*!
\brief Gets the state of the worker
*/
bool isRunning() { return _thread.isRunning(); }

private slots:
void stop();

public slots:
/*!
\brief a slot to indicates the end of our current work
*/
void terminate();

signals:
void finished();
};

#endif





#include "ThreadWorker.h"

ThreadWorker::ThreadWorker(int frequency)
{
_frequency = frequency;
_timer.setInterval(_frequency);

connect(&_thread, SIGNAL(started()), &_timer, SLOT(start()));
connect(this, SIGNAL(finished()), &_thread, SLOT(quit()));
connect(this, SIGNAL(finished()), this, SLOT(stop()));

_timer.moveToThread(&_thread);
}

ThreadWorker::~ThreadWorker()
{
stop();
_thread.quit();
_thread.wait();
}

void ThreadWorker::createWorker(const QObject* source, const char *slot)
{
connect(&_timer, SIGNAL(timeout()), source, slot, Qt::DirectConnection);
}

void ThreadWorker::onWorkerEnded(const QObject* source, const char *slot)
{
connect(&_thread, SIGNAL(finished()), source, slot, Qt::DirectConnection);
}

void ThreadWorker::setFrequency(int frequency)
{
_frequency = frequency;
}

void ThreadWorker::start()
{
if(!_thread.isRunning())
_thread.start();
}

void ThreadWorker::stop()
{
if(_timer.isActive())
{
_timer.stop();
}
}

void ThreadWorker::terminate()
{
if(_timer.isActive())
{
emit finished();
}
}


Again, thank you very much for your help

[yeye_olive]

Here are a few comments:

1. In createWorker(), you make a direct connection between timer and source which live in distinct threads. Qt explicitly forbids that. You should move source to _thread instead (see Manager::add() in my code).

2. In onWorkerEnded(), you make a direct connection again, which is only allowed if source lives in the same thread as the _thread object itself, i.e. presumably the main thread, NOT the one managed by _thread. More generally, I have yet to find a use case for forcing a direct connection: the default behaviour (auto connection) behaves like a direct connection if it is allowed and like a queued connection otherwise.
By the way, why do you even need onWorkerEnded()?

3. In ThreadWorker::stop(), you call _timer.stop() but _timer lives in another thread and QTimer's documentation explicitly states that a QTimer must be started/stopped from the thread in which it lives. That is why in my code I start the timer with

QObject::connect(&thread, SIGNAL(started()), &timer, SLOT(start()));

instead of

timer.start();

In the same fashion you should stop the timer this way:

QMetaObject::invokeMethod(&_timer, "stop");

4. It seems that you do not really need the ThreadWorker::finished() signal and the ThreadWorker::stop() slot. You could simply replace the body of ThreadWorker::terminate() with

QMetaObject::invokeMethod(&_timer, "stop");
_thread.quit();

but I do not know if QTimer::stop() will always be executed before _thread's event loop terminates (IMHO the documentation is a bit lacking wrt this very specific point). If I wanted to ensure that, I would invoke a slot for a custom QObject living in the worker thread that would stop the timer (direct method call), then make the event loop exit (not a direct call).

I understand this whole business can be really confusing. I strongly recommend that you read the documentation on signals and slots across threads, QThread, and QTimer.

[amleto]

ocsidot, unless you are experienced Qt programmer there is just no need to be be using Qt::DirectConnection explicitly.

So my advice is to just stop it!