Problem in Multithreading Application

vidar

Hi,

I have two threads:

the main application containing the GUI
and a second one that I use to update the values of an array, whenever settings in the GUI change.

To notify the worker thread, I emit a signal that is connected with a slot of the worker's thread:

@connect(this, SIGNAL (updateArr(QPainterPath)),
this->threadUpdater,SLOT (updateArr(QPainterPath)), Qt::QueuedConnection);@

From the Qt Documentation I expected the slot to be executed in the worker's thread. I am using Mutexes to ensure, that accessing member values is safe:

@void ThreadUpdater::updateArr(const QPainterPath &bezPath) {

mutexUpdate->lock();
doSomething(bezPath);
mutexUpdate->unlock();
}@

However, when debugging the Application, I get the warning: "A mutex must be unlocked in the same thread that locked it.").

It seems that, the slot is not called from within the worker's thread, as I expected... What am I doing wrong?

The worker's thread does install an event loop:
@void ThreadUpdater::run() {
mutexUpdate = new QMutex();
exec();
delete mutexUpdate;
}

void ThreadUpdater::stopThread() {
quit();
}@

vsorokin

Please, show, how you create working thread object (ThreadUpdater).

andre

Seems like that thread is constructed wrong. Read "this article":http://developer.qt.nokia.com/wiki/ThreadsEventsQObjects on the wiki please.

giesbert

[quote author="vidar" date="1318347835"]Hi,
To notify the worker thread, I emit a signal that is connected with a slot of the worker's thread:

@connect(this, SIGNAL (updateArr(QPainterPath)),
this->threadUpdater,SLOT (updateArr(QPainterPath)), Qt::QueuedConnection);@

From the Qt Documentation I expected the slot to be executed in the worker's thread. I am using Mutexes to ensure, that accessing member values is safe:
[/quote]

This is only true, if you moved the thread object to be attached to the thread itself. Each QObject has a thread affinity to the thread that created it. The QThread object itself is a QObject that hosts a thread, but its affinity is towards its creator thread. Tho change that, use moveToThread.

andre

I don't think that using moveToThread on the thread object itself is very good advice. Instead, it would be better to start using QThread like this:

@
MyWorkerObject* worker = new MyWorkerObject();
QThread* workerThread = new QThread(this);
worker->moveToThread(workerThread);
connect(this, SIGNAL(someSignal()), worker, SLOT(someSlot()));
workerThread->start();
@

That is: use a vanilla QThread (not a subclass), and use a QObject-derived worker object that you move to the worker thread object.

vidar

Thank you! I think I still need to meditate about the "the QThread object does not live in the Thread itself" - thing. But from the wiki article I read that it is a good way to connect the signal & slot from within the run() method of the QThread... Or to use the code that Andre posted. I will check it out.
Thanks a lot!

andre

To help you in your meditation: you should understand that QThread is an object that manages a thread, but it is not the thread itself. A thread is not an object, it is a path of code execution that runs parallel to another code execution path.

giesbert

[quote author="vidar" date="1318367955"]But from the wiki article I read that it is a good way to connect the signal & slot from within the run() method of the QThread... [/quote]

That would be even better and could remove the moveToThread thing.

Create a subclass of QThread and add some slots

connect the outside class with the slost of your thread object

you QThread should emit a signal for each slot

inside run, create the worker qobjects and connect them to your threads signals.

This decouples the worker objects from the outer world, removes the moveToThread stuff and from outside, it looks (and it also does) that the thread handles the stuff.

This is just another idea how to solve that.

andre

In that case, I would remove the QThread itself from view. Don't subclass it, but instead just subclass a QObject that you give the nessecairy API, and let that class care about what way of making the operations async it wants to use. That could be QThread, it could be not. I would really advice against subclassing QThread and adding slots and/or signals to it.

goetz

Wasn't that subclassing QThread exactly that "'You're doing it wrong'":http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/ thing?

giesbert

If you move the thread object, YES. But that wasn't my suggestion. My idea was to use it as multiplexer so you can create the target QObjects inside run and connect the signals there.

goetz

In that case, I would recommend creating a plainly QObject derived class that keeps a QThread as member variable (or pointer). This way the implementation details do not leak to the outside, as the users of the class should not be exposed with the QThread methods directly.

andre

That's what I suggested as well:
[quote author="Andre" date="1318490455"]In that case, I would remove the QThread itself from view. Don't subclass it, but instead just subclass a QObject that you give the nessecairy API, and let that class care about what way of making the operations async it wants to use.[/quote]

Do not subclass just because it is convenient. Subclass because your object is (a specialized version of) the class you are subclassing and you intend to use it that way. In this case, encapsulation seems a much better solution.

vidar

Hi,

looking at the philosophy of object-oriented design, Andre's and Volker's solution seems to be more "correct".
However, I somehow prefer the dataflow-oriented solution that Gerolf mentioned (subclassing of QThread and using it as a "proxy"), for the following reason: the overall application is clearly partitioned into "domains" in which things are done in parallel. The borders of these domains are the instances of the QThread subclasses and thus can be identified easily.

mamtamehra

This post is deleted!