[Solved] Completely lost the QThread plot...

idlefrog

Hello All,

I did think I understood threads. But not now. I have a class called MyThread derived from QThread.
I am trying to get the run method of my MyThread object to call an object's QProcess::start method.

However, I get this error:
Cannot create children for a parent that is in a different thread
(Parent is QProcess(0x6ede80), parent's thread is QThread(0x64a320), current thread is MyThread(0x7fffffffe3c8)
This is saying that QProcess lives in the GUI thread, and so MyThread can't call start on it.

If I create the QProcess object in the run method, then I get a similar error:
QObject: Cannot create children for a parent that is in a different thread.
(Parent is MyThread(0x7fffffffe3c8), parent's thread is QThread(0x64a320), current thread is MyThread(0x7fffffffe3c8)
This is saying that although QProcess was created in the QThread's run method, the parent of MyThread is the GUI thread.
So you are not allowed to create QPrrocess inside the run method either.

So creating the QProcess within the GUI thread OR the QThread::run method is NOT ALLOWED.

Just to repeat this: You can not create QProcess objects outside the GUI thread because MyThread was created in the GUI Thread, and you can not create it inside the run method because the parent of MyThread is the GUI thread.

Yes, I've lost the plot! HELP!

Chris Kawa

Like many others you have fallen into a trap of subclassing QThread. This is exactly the reason why you should use worker objects instead of subclassing. Follow this example to see how to use a worker object.

To explain what went wrong:

First you created a QThread object (lets call it t). It lives in the UI thread. When you start it another thread is spawned and a run method is executed in it. run creates a QProcess object (lets call it p) and you are trying to give it t as a parent. That's an error because they live in different threads:

UI Thread	The other thread
`t = new QThread()`
`t->start()`	`t::run()` is called here
	`p = new QProcess(t)` nope, can't do

Using a worker object (lets call it wo) this is how it works:

UI Thread	The other thread
`t = new QThread()`
`wo = new Worker()`
`wo->moveToThread(t)` ==>	`wo` now lives here!
`connect(t, &QThread::started, wo, &Worker::doWork)`
`t->start()`	`t::run()` is called here and just starts an event loop
	`wo::do Work()` is called here
	`p = new QProcess(wo)` yup, this works

The other way to avoid this whole mess is not to give the QProcess the thread object as a parent. Create a parentless process object and connect its destruction (deleteLater method) to the finished signal of the process or make it a parentless stack variable. Then it simply becomes:

Thread	The other thread
`t = new QThread()`
`t->start()`	`t::run()` is called here
	`QProcess p;` sure, why not

idlefrog

Thank you Chris,

Your example code worked like a dream :) It's most kind of your to explain what's going on.
And, yes, I could have used a parentless process as you suggested, but then I guess it is not possible to react to its signals.

My head has stopped spinning now :)

Thanks again!

Chris Kawa

Glad I could help.

You can connect to parentless process object just the same. Having a parent doesn't change anything in that regard.

idlefrog

Reading the QThread documentation it doesn't explicitly say don't subclass QThread.
In fact it seems to give it as an option.

I tried the Worker object solution, and that worked very well.
But I also wanted to try a QThread subclass solution too. After a little thought, I came up with something like this:
In the GUI thread I did this:
MyThread *wo = new MyThread() ;
wo->setTasks( &m_tasks ); // tasks are QObjects that run a QProcess
wo->start();

The MyThread run method looks pretty simple:
void MyThread::run()
{
foreach( Task t, *m_tasks ){ t.run() ; } // Task objects here belong to the new thread.
}

And then in the Task class we can call QProcess, like this:
void Task::run()
{
m_p->start("bash", QStringList()<< "-c" << m_cmd ) ; // run bash with some command for example
m_p->waitForFinished() ;
}
where m_p = new QProcess() ; // Somewhere in the constructor

Although MyThread does not have an event loop, it does connect the signals from QProcess to the Task's slots.
Thanks for your help, ( as sometimes its easy to get caught up in a mental rut ).

Chris Kawa

It's not that you should never subclass QThread. It's just you should know what it entails .

Be careful with the above approach. It still has the same issue as before. Task objects (like the thread object) live in the UI thread. It works because you don't give the m_p the task object as a parent, but if you try that it will error out like before.

To make it more bulletproof I would recommend that setTasks() took ownership of the task objects (i.e. deleted them when done) and used moveToThread() to move them to the worker thread. This way they can become a parent for objects it carries if the need arises.

The other benefit of that is that if you connected QProcess to such moved task it would use Qt::DirectConnection and executed task's slots in the worker thread, which offloads the UI thread. In your approach the task lives i the UI thread so the connection becomes Qt::QueuedConnection and tasks slots are executed in the UI thread.

idlefrog

Yes, you are right that the Tasks in QList live in the UI thread. But in the MyThread::run() method they are copied each time in the foreach loop.
So the Task lives in the MyThread thread, and the QProcess created by the Task object is also in lives in the MyThread object.
When I make put:
m_p = new QProcess(this) ;
Into the constructor there isn't a problem, as the task has already been copied while in the MyThread thread.

Thanks for reviewing my comments. As I'm only just starting to get to grips with QThread and thread affinity etc.

BTW I did double check what thread was calling my Task slot, both the affinity and current thread were of MyThread.

Chris Kawa

Ok, fair enough, I didn't notice the copy. But you said tasks are QObjects. QObjects are non-copyable types so how does that work exactly? Wouldn't it be better to avoid the copy?

Btw. it starts to look like re-implementing what QtConcurrent::run does. Have you considered using that?

idlefrog

Perhaps I was not clear about my Task class, but it looks like this:

class Task: public QObject
{
    Q_OBJECT

public:

    Task( const char *s, QObject* p = nullptr ) ;
    Task( const Task& t, QObject *p = nullptr ) ;

    Task& operator=( const Task& t){ m_cmd = t.m_cmd ; return *this ;}

Yes, I see what you mean about QtConcurrent as it works on lists, and I have a list of Tasks. So I could run a function on each task and in parallel (and get a performance boost). But for now I will run them in sequence.

Thanks once again for your help! :)

JKSH

@idlefrog said:

Yes, I see what you mean about QtConcurrent as it works on lists, and I have a list of Tasks. So I could run a function on each task and in parallel (and get a performance boost). But for now I will run them in sequence.

QtConcurrent::map() is your friend :-)