How to detect QObject::moveToThread() failure in Qt5?
Asked Answered
G

2

5

The documentation on QObject::moveToThread() for Qt5.3 explains that the moveToThread() method can fail if the object has a parent. How would I detect this failure in my code?

I realize that simply making sure that my object does not have a parent first is probably good enough, but as a defensive programming practice I would like to test the return value from all calls that may fail.

EDIT: I want to stress here after some answers that I am fully aware that I can test if parent is 0 before calling moveToThread. I am looking for possible ways to determine empirically that the moveToThread call actually succeeded.

Gerkman answered 1/11, 2014 at 20:2 Comment(1)
Perhaps by overloading QThread?Cyrillus
C
9

To reliably get the result of moveToThread(), catch the ThreadChange event of the object undergoing the move (by overriding QObject::event() or installing an event filter), and store whether the event has been seen in a reference to a local variable:

 static bool moveObjectToThread(QObject *o, QThread *t) {
     class EventFilter : public QObject {
         bool &result;
     public:
         explicit EventFilter(bool &result, QObject *parent = nullptr)
             : QObject(parent), result(result) {}
         bool eventFilter(QObject *, QEvent *e) override {
             if (e->type() == QEvent::ThreadChange)
                 result = true;
             return false;
         }
     };
     bool result = false;
     if (o) {
         o->installEventFilter(new EventFilter(result, o));
         o->moveToThread(t);
     }
     return result;
 }

Long story:

  1. The documentation is wrong. You can move a QObject with a parent to another thread. To do so, you just need to call moveToThread() on the root of the QObject hierarchy you want to move, and all children will be moved, too (this is to ensure that parents and their children are always on the same thread). This is an academic distinction, I know. Just being thorough here.

  2. The moveToThread() call can also fail when the QObject's thread() isn't == QThread::currentThread() (ie. you can only push an object to, but not pull one from another thread).

  3. The last sentence is a lie-to-children. You can pull an object if it has before been dissociated with any thread (by calling moveToThread(nullptr).

  4. When the thread affinity changes, the object is sent a QEvent::ThreadChange event.

Now, your question was how to reliably detect that the move happened. The answer is: it's not easy. The obvious first thing, comparing the QObject::thread() return value after the moveToThread() call to the argument of moveToThread() is not a good idea, since QObject::thread() isn't (documented to be) thread-safe (cf. the implementation).

Why is that a problem?

As soon as moveToThread() returns, the moved-to thread may already have started executing "the object", ie. events for that object. As part of that processing, the object might be deleted. In that case the following call to QObject::thread() on the original thread will dereference deleted data. Or the new thread will hand off the object to yet another thread, in which case the read of the member variable in the call to thread() in the original thread will race against the write to the same member variable within moveToThread() in the new thread.

Bottomline: Accessing a moveToThread()ed object from the original thread is undefined behaviour. Don't do it.

The only way forward is to use the ThreadChange event. That event is sent after all failure cases have been checked, but, crucially, still from the originating thread (cf. the implementation; it would also be just plain wrong to send such an event if no thread change actually happened).

You can check for the event either by subclassing the object you move to and reimplementing QObject::event() or by installing an event filter on the object to move.

The event-filter approach is nicer, of course, since you can use it for any QObject, not just those you can or want to subclass. There's a problem, though: as soon as the event has been sent, event processing switches to the new thread, so the event filter object will be hammered from two threads, which is never a good idea. Simple solution: make the event filter a child of the object to move, then it will be moved along with it. That, on the other hand, gives you the problem how to control the lifetime of the storage so you can get the result even if the moved object is immediately deleted when it reaches the new thread. To make a long story short: the storage needs to be a reference to a variable in the old thread, not a member variable of the object being moved or the event filter. Then all accesses to the storage are from the originating thread, and there are no races.

But, but... isn't that still unsafe? Yes, but only if the object is moved again to another thread. In that case, the event filter will access the storage location from the first moved-to thread, and that will race with the read access from the originating thread. Simple solution: de-install the event filter after it has fired once. That implementation is left as an exercise to the reader :)

Cystolith answered 1/1, 2015 at 21:1 Comment(7)
I think you are mistaking in some parts. I don't think the docs are wrong, it's the perspective that you are looking at things that's wrong. You can NOT move an object which has a parent to another thread. In your example you are moving the root object which is the parent and has no parent, not the object which has a parent. In other words you are moving an entire hierarchy of objects with the scope of moving just one which I don't think it's what you want. What you say is that moving a parent less object to another thread moves also its children but this is another thing.Raeraeann
"this is to ensure that parents and their children are always on the same thread" - parents and their children CAN'T be on different threads.Raeraeann
ThreadChanged is the last event sent to this object in the previous thread. This means that when ThreadChanged is received, the object is still in the previous thread. So how can you be sure that after ThreadChanged is sent, the object doesn't fail to move?...Chlorite
@Jakob Krieg: a) because I read the code and b) it would be wrong to send a Thread_Change_ event if there was no thread changed :)Cystolith
@luliu: sorry, I don't get your comments. Regarding the first: I stated myself that the distinction is academic. Regarding the second: if an object with a parent could be moved, then the moved child would be on a different thread than its parent. Thus, forbidding moving children without at the same time moving their parents does therefore ensure that they do stay on the same thread. I stand by my wording.Cystolith
@MarcMutz-mmutz Please correct me if I'm wrong. I think using QObject::thread() is a good idea. Why comparing the QObject::thread() return value after the moveToThread() call to the argument of moveToThread() is not a good idea if you use thread synchronization mechanisms? And why is moveObjectToThread thread safe since most of the operations done inside it are not thread safe?Raeraeann
@Iuliu: I've expanded my answer.Cystolith
R
2

QObject::moveToThread fails only if it has a parent. If its parent is NULL then you can move it, else you can't.

EDIT:

What you could do is you can check the object's thread affinity after you called moveToThread by calling QObject::thread and checking if it had really changed its affinity.

QThread *pThread = new QThread;

QObject *pObject = new QObject;

{
    QMutexLocker locker(&mutex);

    pObject->moveToThread(pThread);

    if(pObject->thread() != pThread)
    {
        qDebug() << "moveToThread failed.";
    }
}
Raeraeann answered 1/11, 2014 at 20:7 Comment(9)
As I stated in the question, I am aware of this. I am looking for how to test if it failed afterwards.Gerkman
I understand that I can check FIRST if there is a parent and then PRESUME it worked if parent was 0. However crazy stuff happens in any multi threaded software and feeling adequately paranoid I want empirical evidence that it in fact did not fail.Gerkman
I hear you, but still, in my years as a developer I have encountered my share of strage and unexpected errors, most of which would have been avoided simply by checking return values and verifying the most basic assumptions. I have developed this into a practice and it serves me well. Further, I would argue that moveToThread() is a quite crucial function in Qt5. Due to the basic architecture of Qt, actually knowing for sure that code is actually running in a thread is a big deal as the alternative is that it will (unexpectedly) just continue to run in main thread.Gerkman
@LennartRolland If you have arranged your code and threads such that you have a race condition, and if (!obj->parent()) {obj->moveToThread(...);} can fail, you'd still have a horrible race condition if moveToThread() provided a return value, and it would make no difference.Maximomaximum
Our application is embedded. It runs on inaccessible hardware exposed to random users out in the wild (vending machine). We take pride in the fact that our software never fails. The software itself is quite simple and we have not noticed any problem with performance even after handling every bit of return value very seriously. Clearly you are unable to my question.Gerkman
I +1 it, but waiting before accepting this til I get a chance to verify it, and also giving other a chance to submit alternate answers. (I am writing a set of unit test for my thread code right now that I hope will prove useful)Gerkman
WARNING: do not use the above code in production. It has undefined behaviour. QObject::thread() is not thread-safe.Cystolith
@MarcMutz-mmutz I edited the post; using a mutex would solve the thread-safety problem. I gave the example as a reference. The thread affinity can be checked using QObject::thread(), it is up to the user to do the synchronization mechanism; speaking of safety, I don't think the moveObjectToThread function in your example is thread-safe either.Raeraeann
@Iuliu: moveObjectToThread() does not need to be thread-safe. Cf. my answer for details.Cystolith

© 2022 - 2024 — McMap. All rights reserved.