[Qt-interest] Question about the thread affinity of a QThread object

[K. Frank]

Sean -

Thanks.

On Fri, May 21, 2010 at 4:53 AM, Sean Harmer
<sean.harmer at maps-technology.com> wrote:
> ...
> On Thursday 20 May 2010 23:29:36 K. Frank wrote:
>> ...
>> Now, I _think_ I have seen warnings, maybe in the documentation, or maybe
>> in a list discussion, that one should not emit signals from a QThread's
>> run() method.
>
> Emitting signals from a QThread object (even in its run() function) is fine. I
> think the warnings you are referring to were about not putting *slots* in the
> QThread object and instead having worker objects created in run() to do the
> work.

Okay, I think I understand the concern.  Let me see if i can concoct an
example that illustrates this:

   class Receiver : public QThread {
   Q_OBJECT
      public:
         Receiver() : i_(0), j_(0) {}
         void run() {
            j_ = i_ + 1;  // manipulate the private data
            exec();
         }
      public slots:
         void receive() { i_ = j_+1;)  // let an outside signal
manipulate the data
      private:
         int i_, j_;  // some dummy data to manipulate
   };

   class Sender : public QObject {
   Q_OBJECT
      public:
         void emit_send() { emit send(); }  // not really necessary?
      public signals:
         void send();
   };

   // the main gui thread, call it t0

   s = new Sender;   // thread affinity t0, but irrelevant
   t1 = new Receiver;   // thread affinity t0 (not t1)
   connect (s, SIGNAL(send()), t1, SLOT(receiver()));
   t1->start();
   s->emit_send();  // danger of race condition manipulating t1's data

   // ...

I would like to check my understanding:

The connect call creates a Qt::AutoConnection between s and t1.
Therefore how t1->receive() is called (synchronous call or posted
event) depends on the thread of execution that emits the signal
and the thread affinity of the receiver.

In this case, the signal, s->send() is emitted by thread t0, which
is also the thread for with the receiver, t1, has affinity.  Therefore
the slot, t1->receive(), is called synchronously by thread t0,
essentially as if "s->emit-signal();" had been replaced by
"t1->receive();".

The race condition occurs because two different threads, t0 and t1,
are monkeying around with t1's private data:  t0 through the
signal / slot connection, and t1 through the line of code
"j_ = i_ + 1;" in t1's run() method.

(The call to exec() in t1->run() is, in a sense, a red herring.  The
direct connection bypasses exec(), and t1->receive() would be
called, even if exec() had been left out.)

Do I have this example (and its analysis) right, and does it
illustrate the potential pitfall with having QThread's implement
slots?

> It's not that you can't do this, just that it is clearer in which thread
> context the slots will get executed. Remember that the QThread object itself
> has affinity with its parent object which lives in the thread which created
> the QThread object in question - unless you explicitly move it of course.

So, can one "fix" the example -- eliminate the race condition -- by moving
t1 to itself?

   t1 = new Receiver;   // thread affinity t0 (not t1)
   t1->moveToThread (t1);   // now t1 has thread affinity t1

Is it legal (and moral) to do this in Receiver's constructor?

   Receiver::Receiver() : i_(0), j_(0) {
      moveToThread (this);
   }

I _think_ that this would be officially legitimate from a C++
perspective.  Is it consistent with Qt's inner workings?

> To avoid confusion when it comes to maintenance time 2 years down the line and
> the new employee fails to spot the importance of your moveToThread() call and
> removes it then bad things will happen. If you explicitly create worker
> objects in QThread::run() it becomes much clearer to see that the slots of
> these objects will be executed in the context of your QThread object and not
> the thread your QThread object has affinity to.

Agreed.  From a readability and maintainability standpoint (and probably
also from the perspective of separation of concerns), it's better to just
let QThread control a thread of execution, and use additional worker
objects to implement the processing logic.

>> ...
>
> The connection type to use (queued/direct etc) is determined when you call
> emit (unless you force it to something else in your connect call). The signal
> is being emitted from within t2's context and is connected to t1 which has
> affinity with t1 and so a queued connection is used.

I would like to verify my understanding of this;  (I think Thiago said
essentially
the same thing earlier.)

When connect is called with Qt::AutoConnection (the default), the type of
connection is determined not when connect is called, but rather, on a
call-by-call basis when emit signal is called.

So, in my example:

   s = new Sender;   // thread affinity t0, but irrelevant
   t1 = new Receiver;   // thread affinity t0 (not t1)
   connect (s, SIGNAL(send()), t1, SLOT(receiver()));  // defaults to
AutoConnection
   t1->start();

   s->emit_send();  // direction-connection; thread t0 calls t1->receive()
   t1->moveToThread (t1);   // now t1 has thread affinity t1
   s->emit_send();  // queued-connection; thread t1 calls t1->receive()

Is this right?

>> ...
> ...
>> If t2's thread affinity is irrelevant to how the signal is propagated, and
>> therefore the signal / slot connection is (properly) a queued connection,
>> what are all the warnings about?
>
> I think you imagined them wrt emitting signals from a thread. I do not think
> emitting signals form a thread is a problem. The Mandelbrot example does this
> so even the trolls do it :-)

Yes, I think that clears things up.  I remembered correctly that there were
warnings, but it hadn't sunk in exactly what those warnings were about.

Thanks for your explanations.

K. Frank

> ATB,
>
> Sean