Discussion:
[Interest] Is it OK to emit from different thread?
Alexander Dyagilev
2018-11-30 18:53:19 UTC
Permalink
Hello,

Let's suppose some QObject derived class belongs to thread A. It has
some method, which emits some signal.

This method may be called from another thread B. Thus, signal will be
issued for this object from the thread it does not belong to.

Is it OK?

source code (just in case):

class MyObject {
Q_OBJECT
signals:
void mySignal();
public:
void test();
}

void MyObject::test()
{
     emit mySignal();
}

// thread A:
...
auto obj = new MyObject();
...

// thread B:
...
obj->test();
...
Konstantin Tokarev
2018-11-30 19:14:23 UTC
Permalink
Post by Alexander Dyagilev
Hello,
Let's suppose some QObject derived class belongs to thread A. It has
some method, which emits some signal.
This method may be called from another thread B. Thus, signal will be
issued for this object from the thread it does not belong to.
Is it OK?
Given that "emit" is no-op keyword and emission of signal is just a method call,
it's fine to do this. However, exact behavior will depend on how particular slots
are connected to that signal: slots with direct connection will run in thread B,
not A.
Post by Alexander Dyagilev
class MyObject {
Q_OBJECT
void mySignal();
void test();
}
void MyObject::test()
{
      emit mySignal();
}
...
auto obj = new MyObject();
...
...
obj->test();
...
_______________________________________________
Interest mailing list
https://lists.qt-project.org/listinfo/interest
--
Regards,
Konstantin
Marian Beermann
2018-11-30 19:35:21 UTC
Permalink
Note that the default is "AutoConnection", which...
(Default) If the receiver lives in the thread that emits the signal,
Qt::DirectConnection is used. Otherwise, Qt::QueuedConnection is used.
The connection type is determined when the signal is emitted.

Cheers, Marian
Post by Alexander Dyagilev
Hello,
Let's suppose some QObject derived class belongs to thread A. It has
some method, which emits some signal.
This method may be called from another thread B. Thus, signal will be
issued for this object from the thread it does not belong to.
Is it OK?
Given that "emit" is no-op keyword and emission of signal is just a method call,
it's fine to do this. However, exact behavior will depend on how particular slots
are connected to that signal: slots with direct connection will run in thread B,
not A.
Post by Alexander Dyagilev
class MyObject {
Q_OBJECT
void mySignal();
void test();
}
void MyObject::test()
{
      emit mySignal();
}
...
auto obj = new MyObject();
...
...
obj->test();
...
_______________________________________________
Interest mailing list
https://lists.qt-project.org/listinfo/interest
Jérôme Godbout
2018-11-30 19:15:40 UTC
Permalink
The fact that QObject belong to Thread A and a method is used into Thread B should raise a flag, each QObject belong to a QThread and they should be used by that QThread only. You should move the Object to the other thread or use a signals / slots to communicate between thread.

You will have many problems the way you describe it, the fact that the thread B is launching the signal of Object that belong on Thread A, the automatic connection will check the ownership of the object emiting the signal to queue or not the signal and this will be wrong since the related thread is not the current thread.

Note: inheriting QThread is wrong practice and should probably never be done. Since the Qthread belong to the thread creator and not the created thread you will have the wrong current thread for the current QObject that inherit QThread.

You can see this article that explain the problem by inheriting the QThread: http://blog.debao.me/2013/08/how-to-use-qthread-in-the-right-way-part-1/

The Qthread doc and example should really be updated if they still show inheritance of QThread, many green developer fall into the pit every time and I can understand if the doc still do this.

-----Original Message-----
From: Interest <interest-***@lists.qt-project.org> On Behalf Of Alexander Dyagilev
Sent: November 30, 2018 1:53 PM
To: ***@qt-project.org
Subject: [Interest] Is it OK to emit from different thread?

Hello,

Let's suppose some QObject derived class belongs to thread A. It has some method, which emits some signal.

This method may be called from another thread B. Thus, signal will be issued for this object from the thread it does not belong to.

Is it OK?

source code (just in case):

class MyObject {
Q_OBJECT
signals:
void mySignal();
public:
void test();
}

void MyObject::test()
{
     emit mySignal();
}

// thread A:
...
auto obj = new MyObject();
...

// thread B:
...
obj->test();
...

_______________________________________________
Interest mailing list
***@lists.qt-project.org
https://lists.qt-project.org/listinfo/interest
Konstantin Tokarev
2018-11-30 19:46:31 UTC
Permalink
30.11.2018, 22:40, "Jérôme Godbout" <***@amotus.ca>:

<snip>
Post by Jérôme Godbout
Note: inheriting QThread is wrong practice and should probably never be done.
This advise is questionable

https://woboq.com/blog/qthread-you-were-not-doing-so-wrong.html

<snip>
--
Regards,
Konstantin
Jérôme Godbout
2018-11-30 20:07:42 UTC
Permalink
If you use automatic connection when doing this you will run into troubles and my be surprise by the signals sent from the thread execution (they will act like if they were sent from the Qthread creator and not the actual executing qthread and this is bad.

If you see some code where people tell they have to specify the queued connection manually, it smell like a QThread have been inherited to inline the run and they "fixed" the problem. I do a lot of Threading inside my application and I never inherit it and never had any problems with my thread either.

-----Original Message-----
From: Konstantin Tokarev <***@yandex.ru>
Sent: November 30, 2018 2:47 PM
To: Jérôme Godbout <***@amotus.ca>; Alexander Dyagilev <***@gmail.com>; ***@qt-project.org
Subject: Re: [Interest] Is it OK to emit from different thread?



30.11.2018, 22:40, "Jérôme Godbout" <***@amotus.ca>:

<snip>
Post by Jérôme Godbout
Note: inheriting QThread is wrong practice and should probably never be done.
This advise is questionable

https://woboq.com/blog/qthread-you-were-not-doing-so-wrong.html

<snip>

--
Regards,
Konstantin
Konstantin Shegunov
2018-11-30 21:05:13 UTC
Permalink
Post by Jérôme Godbout
If you use automatic connection when doing this you will run into troubles
and my be surprise by the signals sent from the thread execution (they will
act like if they were sent from the Qthread creator and not the actual
executing qthread and this is bad.
No they will not. They will act just like they've been sent from the thread
that does the emit.

If you see some code where people tell they have to specify the queued
Post by Jérôme Godbout
connection manually, it smell like a QThread have been inherited to inline
the run and they "fixed" the problem. I do a lot of Threading inside my
application and I never inherit it and never had any problems with my
thread either.
You don't need to use Qt::QueuedConnection when inheriting QThread and
emitting from QThread::run, and I've done that aplenty. There's no problem
with it, as long as you realize the QThread object is in the thread that
created it - i.e. its internal data is not owned by QThread::run. With auto
connection Qt will do the right thing and queue the slot call if the
receiving thread is different from QThread::run (which it really almost
always is) and to be really explicit this determination whether to queue,
or to call directly is done at the time of emit (more concretely in
QMetaObject::activate and its friends) and not at time of making the
connections.

Running imperative code in a subclassed thread is perfectly fine. Not every
thread needs an event loop.
Jérôme Godbout
2018-11-30 21:34:45 UTC
Permalink
While I agree with your point, it’s easy to shoot yourself in the foot as soon as one add a slots into that QThread inherited class (can see the kdab recommendation on the subject point #5: https://www.kdab.com/nailing-13-signal-slot-mistakes-clazy-1-3/ ). Your green developer will be tempted to add a slots into this object if they feel they need a slots and there the fun will be over. Having to specify your connect() not auto is very rare.

From: Konstantin Shegunov <***@gmail.com>
Sent: November 30, 2018 4:05 PM
To: JérÎme Godbout <***@amotus.ca>
Cc: Konstantin Tokarev <***@yandex.ru>; Alexander Dyagilev <***@gmail.com>; Interests Qt <***@qt-project.org>
Subject: Re: [Interest] Is it OK to emit from different thread?

On Fri, Nov 30, 2018 at 10:49 PM JérÎme Godbout <***@amotus.ca<mailto:***@amotus.ca>> wrote:
If you use automatic connection when doing this you will run into troubles and my be surprise by the signals sent from the thread execution (they will act like if they were sent from the Qthread creator and not the actual executing qthread and this is bad.

No they will not. They will act just like they've been sent from the thread that does the emit.
If you see some code where people tell they have to specify the queued connection manually, it smell like a QThread have been inherited to inline the run and they "fixed" the problem. I do a lot of Threading inside my application and I never inherit it and never had any problems with my thread either.

You don't need to use Qt::QueuedConnection when inheriting QThread and emitting from QThread::run, and I've done that aplenty. There's no problem with it, as long as you realize the QThread object is in the thread that created it - i.e. its internal data is not owned by QThread::run. With auto connection Qt will do the right thing and queue the slot call if the receiving thread is different from QThread::run (which it really almost always is) and to be really explicit this determination whether to queue, or to call directly is done at the time of emit (more concretely in QMetaObject::activate and its friends) and not at time of making the connections.

Running imperative code in a subclassed thread is perfectly fine. Not every thread needs an event loop.
Konstantin Shegunov
2018-11-30 21:50:22 UTC
Permalink
Post by Jérôme Godbout
While I agree with your point, it’s easy to shoot yourself in the foot as
soon as one add a slots into that QThread inherited class (can see the kdab
https://www.kdab.com/nailing-13-signal-slot-mistakes-clazy-1-3/ ). Your
green developer will be tempted to add a slots into this object if they
feel they need a slots and there the fun will be over. Having to specify
your connect() not auto is very rare.
Probably, but as it happens it's almost always easier to shoot yourself in
the foot than to avoid it. And while subclassing QThread isn't the best way
99.5% of cases, it's still a valid approach, so it should not be summarily
dismissed.
Alexander Dyagilev
2018-11-30 19:46:45 UTC
Permalink
Post by Jérôme Godbout
The fact that QObject belong to Thread A and a method is used into Thread B should raise a flag, each QObject belong to a QThread and they should be used by that QThread only. You should move the Object to the other thread or use a signals / slots to communicate between thread.
Well, I always do so. But this object is a special case. It's not used
by other threads. It uses threads to perform various asynchronous tasks
using lambdas, called from these threads, and these lambdas are supposed
to return some result to it.
Post by Jérôme Godbout
You will have many problems the way you describe it,
No, I will not in this particular case :)
Post by Jérôme Godbout
the fact that the thread B is launching the signal of Object that belong on Thread A, the automatic connection will check the ownership of the object emiting the signal to queue or not the signal and this will be wrong since the related thread is not the current thread.
Hmm... I.e. if both A and B objects belongs to the same thread and slot
of A is connected to signal of B, then if signal of B is issued from
another thread, then slot of A will be called in this different thread,
not the thread A belongs to? OK, thanks.
Post by Jérôme Godbout
Note: inheriting QThread is wrong practice and should probably never be done.
Yep, I know.
Jérôme Godbout
2018-11-30 20:29:26 UTC
Permalink
Post by Jérôme Godbout
Post by Alexander Dyagilev
No, I will not in this particular case :)
Might be, but I hope nobody implement a QThread inherited class slots or signals into the mix.

As for the runtime check with the queued connection, take care, this is how it's checked:
QObject * const receiver = c->receiver;
const bool receiverInSameThread = QThread::currentThreadId() == receiver->d_func()->threadData->threadId;

if ((c->connectionType == Qt::AutoConnection && !receiverInSameThread)
...

Take care of QThread thread Id might surprise you. I strongly recommend you print the thread id to make sure your use case can and will do what you expect. Only the current thread and the receiver are used to determine the automatic connection queued or not.


-----Original Message-----
From: Alexander Dyagilev <***@gmail.com>
Sent: November 30, 2018 2:47 PM
To: Jérôme Godbout <***@amotus.ca>; ***@qt-project.org
Subject: Re: [Interest] Is it OK to emit from different thread?
Post by Jérôme Godbout
The fact that QObject belong to Thread A and a method is used into Thread B should raise a flag, each QObject belong to a QThread and they should be used by that QThread only. You should move the Object to the other thread or use a signals / slots to communicate between thread.
Well, I always do so. But this object is a special case. It's not used by other threads. It uses threads to perform various asynchronous tasks using lambdas, called from these threads, and these lambdas are supposed to return some result to it.
Post by Jérôme Godbout
You will have many problems the way you describe it,
No, I will not in this particular case :)
Post by Jérôme Godbout
the fact that the thread B is launching the signal of Object that belong on Thread A, the automatic connection will check the ownership of the object emiting the signal to queue or not the signal and this will be wrong since the related thread is not the current thread.
Hmm... I.e. if both A and B objects belongs to the same thread and slot of A is connected to signal of B, then if signal of B is issued from another thread, then slot of A will be called in this different thread, not the thread A belongs to? OK, thanks.
Post by Jérôme Godbout
Note: inheriting QThread is wrong practice and should probably never be done.
Yep, I know.
Konstantin Shegunov
2018-11-30 21:09:51 UTC
Permalink
Post by Jérôme Godbout
Only the current thread and the receiver are used to determine the
automatic connection queued or not.
As I wrote in my previous mail. Of course only those two things are
checked, they are the only relevant ones.
Did you expect the sender to be checked as well?
Jérôme Godbout
2018-11-30 22:16:19 UTC
Permalink
It could have been resolved at runtime time when connect() is perform instead and not when called with both source and receiver QObject thread affinity. But that would have leave the moveTothread() the burden to disconnect and reconnect and other mind puzzle somewhere else I’m pretty sure. If you need to known the underhood implementation to perform the correct call, it’s because something need to be clarify somewhere.

The fact that the signal is routed when emitted and the route is not pre traced when doing the connection is required to make the correct assumption.

From: Konstantin Shegunov <***@gmail.com>
Sent: November 30, 2018 4:10 PM
To: JérÎme Godbout <***@amotus.ca>
Cc: Alexander Dyagilev <***@gmail.com>; Interests Qt <***@qt-project.org>
Subject: Re: [Interest] Is it OK to emit from different thread?

On Fri, Nov 30, 2018 at 11:05 PM JérÎme Godbout <***@amotus.ca<mailto:***@amotus.ca>> wrote:
Only the current thread and the receiver are used to determine the automatic connection queued or not.

As I wrote in my previous mail. Of course only those two things are checked, they are the only relevant ones.
Did you expect the sender to be checked as well?

maitai
2018-11-30 19:47:17 UTC
Permalink
Agreed. It took me a long time and pain to realize that deriving from
QThread is a bad idea. Just using connect options, or
QTimer::singleShot(0, etc) to queue it on the wanted thread, is a much
better implementation.

I was really misled by the documentation.
Post by Jérôme Godbout
The fact that QObject belong to Thread A and a method is used into
Thread B should raise a flag, each QObject belong to a QThread and
they should be used by that QThread only. You should move the Object
to the other thread or use a signals / slots to communicate between
thread.
You will have many problems the way you describe it, the fact that the
thread B is launching the signal of Object that belong on Thread A,
the automatic connection will check the ownership of the object
emiting the signal to queue or not the signal and this will be wrong
since the related thread is not the current thread.
Note: inheriting QThread is wrong practice and should probably never
be done. Since the Qthread belong to the thread creator and not the
created thread you will have the wrong current thread for the current
QObject that inherit QThread.
You can see this article that explain the problem by inheriting the
http://blog.debao.me/2013/08/how-to-use-qthread-in-the-right-way-part-1/
The Qthread doc and example should really be updated if they still
show inheritance of QThread, many green developer fall into the pit
every time and I can understand if the doc still do this.
-----Original Message-----
Sent: November 30, 2018 1:53 PM
Subject: [Interest] Is it OK to emit from different thread?
Hello,
Let's suppose some QObject derived class belongs to thread A. It has
some method, which emits some signal.
This method may be called from another thread B. Thus, signal will be
issued for this object from the thread it does not belong to.
Is it OK?
class MyObject {
Q_OBJECT
void mySignal();
void test();
}
void MyObject::test()
{
     emit mySignal();
}
...
auto obj = new MyObject();
...
...
obj->test();
...
_______________________________________________
Interest mailing list
https://lists.qt-project.org/listinfo/interest
_______________________________________________
Interest mailing list
https://lists.qt-project.org/listinfo/interest
Loading...