signal/slot arguments

mzimmers

Hi all -

Is this dangerous?

MyClass myObj = new MyClass;
emit mySignal(myObj);
delete myObj;

And if it is, what's the recommended method of obviating this?

mrjj

Hi
It depends how you have connected the mySignal.
If within same thread, the emit should be like a function call and
it should not crash.

However, if between threads ( Qt::QueuedConnection ) and and Qt saves the pointer and deliver the signal via event loop, it would not be good.

But im wondering why cant the reciever just create a Myclass and use.
Since you delete at once, you seem not to need it where emit is called?

mzimmers

Oh, in my example, I neglected to include the step that populated myObj:

MyClass *myObj = new MyClass;
myObj.myElement = myValue;
emit mySignal(myObj);
delete myObj;

EDIT: fixed mistake in code example.

Basically, I was curious whether the signal/slot mechanism had some built-in protection against a race condition that this code would cause. I think you answered that: if within a thread, it's OK; if not, it's unsafe. Thanks...

mrjj

@mzimmers
Within SAME thread. (like main GUI thread)
then the argument is not stored nothing bad can happen.
Alternatively, you could just send by copy if MyClass is fine with that.
( as in has no char * or anything that would not copy ok)

MyClass myObj;
myObj.myElement = myValue;
emit mySignal(myObj); // would copy
You could change the signal to
void mySignal( MyClass & obj)
and it would transfer as reference. Much like pointer but
no need to delete it. ( scope will delete it)

mzimmers

Yes, that makes sense. Since my example is indeed across threads, though, I have to deal with the potential problem you mentioned.

I think for my purposes, using Qt::BlockingQueuedConnection will work. Either that, or something like slot signaling back an acknowledgement when it's finished, so the sender can then delete the object, but...that sounds like more trouble than it's worth for this application.

wrosecrans

@mzimmers said in signal/slot arguments:

I think you answered that: if within a thread, it's OK

This only applies if you haven't got any queued connections set up. The default behavior of connect is to use queued connections when it sees that two objects are owned by different threads, but it's entirely possible to set up queued connections in other circumstances. In that case, the slot won't trigger until after the object has been deleted, regardless of threading.

Also, if the slot does anything to retain the pointer somewhere rather than just doing some immediate work, obviously that will explode. If possible, avoid sending raw pointers like this, if there's any ambiguity about lifetime and ownership.

You can also look into QObject's delete later : http://doc.qt.io/qt-5/qobject.html#deleteLater

That asks the event loop to delete an object at some point in the future, which is useful if you want it to get deleted after the event loop is done doing some activity with it.

mrjj

@mzimmers
Oh, if between threads, care must be taken.
I normally make sure the class is copyable if not too expensive and
register it to the metasystem so it can cache it.
http://doc.qt.io/qt-5/custom-types.html
As @wrosecrans says, sending pointers is not optimal and add the lifespan and deletion issues. Smart pointers can help but
the best solution really depends on what MyClass contains of data.
BlockingQueuedConnection should work though but i never tested that.

mzimmers

Thanks to both of you for the explanation. I'll avoid passing pointers as arguments in signal/slot calls.

mrjj

@mzimmers
Its not a no go in all cases like when you have a member object in mainwindow and you are sure its life spans matches the application
and you only signal it out for short processing and the pointer is not stored in the using classes. Also sometimes the object creation is so expensive you must use a pointer to avoid copying etc.