[Interest] moveToThread used in constructor to move "this"

[Elvis Stansvik]

2016-02-25 19:10 GMT+01:00 Elvis Stansvik <elvstone at gmail.com>:
> 2016-02-25 18:24 GMT+01:00 Murphy, Sean <smurphy at walbro.com>:
>>> >    Foo() : QObject(nullptr) {
>>> >       moveToThread(&m_thread);
>>> >       m_thread.start();
>>> >    }
>>> >
>>> >    ~Foo() {
>>> >       m_thread.quit();
>>> >       m_thread.wait();
>>> >    }
>>>
>>> This destructor is either never run or deadlocks.
>>>
>>> A QObject can only be destroyed in its thread of affinity. So the above is
>>> running in that m_thread thread, which means it hasn't exited. Waiting for it
>>> to exit will wait forever.
>>
>> Just when I think I have QObjects and QThreads figured out, I realize I don't... How do I correctly handle my situation?
>>
>> I've got a QObject-based class that talks to a custom device and needs to block while making reads/writes to that device. This thread runs the entire duration of my application, because the work it's doing never finishes, it just monitors the device for messages and notifies my UI class when data comes in.
>>
>> So currently I'm doing this:
>>
>> myGUIObject.h:
>> private:
>>   myBlockingObject* blocking;
>>   QThread* thread;
>>
>> myGUIObject.cpp:
>> myGUIObject()
>> {
>>   thread = new QThread(this);
>>   blocking = new myBlockingObject();
>>   blocking->moveToThread(thread);
>> }
>>
>> How/where do I properly delete "blocking"? Right now I'm doing this:
>> ~myGUIObject()
>> {
>>     thread->exit();
>>     blocking->deleteLater();
>> }
>>
>> But Thiago's comment "A QObject can only be destroyed in its thread of affinity" sounds like it's not the right way to do it?
>> Sean
>
> I was in a similar situation some years ago, talking to a USB/serial
> device in a separate thread. I had a look at the code, and what I did
> back then was to connect the finished signal of the thread to the
> deleteLater() slot of the object I had moved to the thread. I actually
> did the same with the thread itself. So the connect() calls I did to
> ensure cleanup looked something like this:
>
>     // Connect to thread signals to start the sensor and clean up.
>     connect(m_sensorThread, SIGNAL(started()), sensor, SLOT(start()));
>     connect(m_sensorThread, SIGNAL(finished()), sensor, SLOT(deleteLater()));
>     connect(m_sensorThread, SIGNAL(finished()), m_sensorThread,
> SLOT(deleteLater()));
>
> Where sensor is the object I had previously moved to m_senseorThread.
>
> Not sure if your approach above with calling deleteLater() explicitly
> and immediately after thread->exit() is right or wrong, or if doing it
> by signal/slot connections like I did is any better. Just thought I
> would chime in :)

I see now that this technique is actually right there in the QThread
docs, under "Managing Thread", third paragraph [1]:

"From Qt 4.8 onwards, it is possible to deallocate objects that live
in a thread that has just ended, by connecting the finished() signal
to QObject::deleteLater()."

So I think this is the approach you should use.

The initial example in the class docs also shows the "suicide"
approach of destructing the thread itself:

    connect(&workerThread, &QThread::finished, worker, &QObject::deleteLater);

In the example you gave, I think you're leaking the thread.

Cheers,
Elvis

[1] http://doc.qt.io/qt-5/qthread.html#managing-threads

>
> Elvis
>
>> _______________________________________________
>> Interest mailing list
>> Interest at qt-project.org
>> http://lists.qt-project.org/mailman/listinfo/interest