SLOT/SIGNAL safety with QByteArray &references

frank.quebec

Hi,

I need an expert's advise on QT thread safety of slot/signal mechanism with Qt::QueuedConnection when passing references.
Let say I have this code

QByteArray byteArray;
QMutexLocker l(&mutex_);
byteArray = reply->readAll();
bool success = true;
connect(this, &Downloader::downloadComplete, client, &DownloadClient::downloadComplete, Qt::QueuedConnection); // Queued is important to make it async and prevent being called in a re-entrant fashion and deadlock itself on mutex if the client's downloadComplete SLOT handler call us again.
emit downloadComplete(success, reply->errorString(), byteArray);
disconnect(this, &Downloader::downloadComplete, client, &DownloadClient::downloadComplete);
return;

// The signal is defined as such in the class .h:

signals:
   void downloadComplete(bool success, const QString& errorString, const QByteArray&);

My question is about the thread safety of sending a reference to QByteArray. Does QT has builtin functionality to make sure it is not erased until nobody has a reference to it ?

Also if anyone has comment on my obligation to add Qt::QueuedConnection to prevent deadlock (it really does deadlock when not marked as such).

Thanks!
Francois

Paul Colby

Hi @frank.quebec,

My question is about the thread safety of sending a reference to QByteArray. Does QT has builtin functionality to make sure it is not erased until nobody has a reference to it ?

The short version is that:

No, Qt doesn't do anything special to guarantee that the reference remains valid - the C++ language itself will define the lifetime of locally (stack) allocated objects, and Qt cannot (should not) change that.
There's no real advantage to using a reference here anyway, since QByteArray is implicitly shared.

Having said that, you would (at least most of the time) get away with using the reference as long as the signal and slot both have the same thread affinity, but not if you're forcing Qt::QueuedConnection.

Your comment on using Qt::QueuedConnection suggests that you using that to get around some other structural issue, which might be worth exploring further. But as far as your question regarding the QByteArray reference, it is indeed unsafe they way you're using it.

// Queued is important to make it async and prevent being called in a re-entrant fashion and deadlock itself on mutex if the client's downloadComplete SLOT handler call us again.

I'm not sure how that's required to make it asynchronous - I can imagine some scenarios, but they're worth avoiding, rather than working around. So there's probably better ways to avoid that deadlock.

So, I'd suggest:

remove the reference from the QByteArray parameter;
explain a bit more about how the above Downloader code, and the DownloadClient::downloadComplete slot are meant to interract, and how that results in a deadlock.

Cheers.

kshegunov

@frank.quebec
Salut Francois,
In general it's not a good idea to send references through a queued connection, even if one leaves the thread safety aside. The problem is that a reference is not much different from a pointer, so when the actual object goes out of scope you will get a segmentation fault accessing a reference to it. For example:

int x;
int & y = x;
{ //< Opening a new stack frame
    int z;
    y = z; //< Switch reference to point to `z`
}
y = 15; //< Crashes, as y references an invalid location

So while most compilers will find the error in the above simple example and will warn you it gets a bit more complicated when working with signals and slots. You can send references, but only provided the original object will not go out of scope. So for most intents and purposes you should be satisfied with just passing objects by value. That's one of the reasons many of the "data" classes in Qt are implicitly shared, and what's more they have internal thread-safe reference counting.

On a side note one of the side effects of using signals and slots is you shouldn't need to use mutexes directly. The signal slot connection (with a few rare exceptions) is safe across thread boundaries. So Just making it is usually enough to ensure race conditions don't occur.

Does QT has builtin functionality to make sure it is not erased until nobody has a reference to it ?

No, but this is outside of Qt's scope. This is because of how C++ works.

Also if anyone has comment on my obligation to add Qt::QueuedConnection to prevent deadlock (it really does deadlock when not marked as such).

You are not. For the most part you will need to use Qt::AutoConnection which is equivalent to using a queued one when working with objects living in different threads, and to direct connection when sender and receiver are in the same thread.

If the above snippet deadlocks, that would pretty strongly suggest you have only one thread and the slot call will double-lock the mutex, thus the program deadlocks.

Kind regards.

Chris Kawa

Actually it's perfectly safe to use const references with Qt::QueuedConnection. This case is handled by Qt in a special way - the parameters are copied so the original objects can go away and it won't harm the connection.

See this blog post for details.

Chris Kawa

It's also worth mentioning that for non-const references (i.e. "out" parameters) it's possible to use a Qt::BlockingQueuedConnection that will make the calling signal block until all slots return. This way the reference remains valid through all slot execution.

I'm mentioning it only because it's possible, but I don't recommend using non-const references in signals/slots. There's no way to guarantee how a user connects them and in case of Qt::QueuedConnection everything @Paul-Colby and @kshegunov said apply.

kshegunov

It seems Chris caught us with our pants down yet again. :D
He's of course correct, that the object will be copied when the meta call is scheduled through the event loop.

Paul Colby

@Chris-Kawa said:

Actually it's perfectly safe to use const references with Qt::QueuedConnection. This case is handled by Qt in a special way - the parameters are copied so the original objects can go away and it won't harm the connection.

Thanks @Chris-Kawa!! It's because of things like that, that I love Qt so much!! :D

See this blog post for details.

Well I've got some fun reading to do... :)

kshegunov

Continues here as a related discussion into QByteArray's data sharing and ownership peculiarities.