I have a scenario where an anonymous QObject
starts an asynchronous operation by emitting a signal. The receiving slot stores the QObject-pointer and sets a property of this object later. The object could be gone meanwhile.
So, is there a safe way to check if this pointer still valid?
P.S.:
I'm aware of QObject::destroyed
signal, which I could connect to the object supposed to call the setProperty
of that pointer. But I wonder, if it works easier.
This is a great question, but it is the wrong question.
Is there a way to check if the pointer is valid? Yes. QPointer
is designed specifically to do that.
But the answer to this question is useless if the object lives in another thread! You only know whether it's valid at a single point in time - the answer is not valid immediately afterwards.
Absent other mechanisms, it is useless to hold a QPointer
to an object in a different thread - it won't help you. Why? Look at this scenario:
Thread A Thread B
1. QPointer returns a non-zero pointer
2. deletes the object
3. Use the now-dangling pointer
I'm aware of QObject::destroyed signal, which I could connect to the object supposed to call the setProperty of that pointer. But I wonder, if it works easier.
The destroyed
signals are useless when sent using queued connections - whether within a thread, or across thread boundaries. They are meant to be used within one thread, using direct connections.
By the time the target thread's event loop picks up the slot call, the originating object is long gone. Worse - this is always the case in a single-threaded application. The reason for the problem is the same as with the QPointer
: the destroyed
signal indicates that the object is no longer valid, but it doesn't mean that it was valid before you received the signal unless you're using a direct connection (and are in the same thread) or you're using a blocking queued connection.
Using the blocking queued connection, the requesting object's thread will block until the async thread finishes reacting to object's deletion. While this certainly "works", it forces the two threads to synchronize on a resource with sparse availability - the front spot in the async thread's event loop. Yes, this is literally what you compete for - a single spot in a queue that can be arbitrarily long. While this might be OK for debugging, it has no place in production code unless it's OK to block either thread to synchronize.
You are trying to work very hard around the fact that you're passing a QObject
pointer between threads, and the object's lifetime, from the point of view of the receiving thread, is uncontrolled. That's your problem. You'd solve everything by not passing a raw object pointer. Instead, you could pass a shared smart pointer, or using signal-slot connections: those vanish whenever either end of the connection is destructed. That's what you'd want.
In fact, Qt's own design patterns hint at this. QNetworkReply
is a QObject
not only because it is a QIODevice
, but because it must be to support direct indications of finished requests across thread boundaries. In light of a multitude of requests being processed, connecting to QNetworkAccessManager::finished(QNetworkReply*)
can be a premature pessimization. Your object gets notified of a possibly very large number of replies, but it really is only interested in one or very few of them. Thus there must be a way to notify the requester directly that its one and only request is done - and that's what QNetworkReply::finished
is for.
So, a simple way to proceed is to make the Request
be a QObject
with a done
signal. When you ready the request, connect the requesting object to that signal. You can also connect a functor, but make sure that the functor executes in the requesting object's context:
// CORRECT
connect(request, &Request::done, requester, [...](...){...});
// WRONG
connect(request, &Request::done, [...](...){...});
The below demonstrates how it could be put together. The requests' lifetimes are managed through the use of a shared (reference-counting) smart pointer. This makes life rather easy. We check that no requests exist at the time main
returns.
#include <QtCore>
class Request;
typedef QSharedPointer<Request> RequestPtr;
class Request : public QObject {
Q_OBJECT
public:
static QAtomicInt m_count;
Request() { m_count.ref(); }
~Request() { m_count.deref(); }
int taxIncrease;
Q_SIGNAL void done(RequestPtr);
};
Q_DECLARE_METATYPE(RequestPtr)
QAtomicInt Request::m_count(0);
class Requester : public QObject {
Q_OBJECT
Q_PROPERTY (int catTax READ catTax WRITE setCatTax NOTIFY catTaxChanged)
int m_catTax;
public:
Requester(QObject * parent = 0) : QObject(parent), m_catTax(0) {}
Q_SLOT int catTax() const { return m_catTax; }
Q_SLOT void setCatTax(int t) {
if (t != m_catTax) {
m_catTax = t;
emit catTaxChanged(t);
}
}
Q_SIGNAL void catTaxChanged(int);
Q_SIGNAL void hasRequest(RequestPtr);
void sendNewRequest() {
RequestPtr req(new Request);
req->taxIncrease = 5;
connect(req.data(), &Request::done, this, [this, req]{
setCatTax(catTax() + req->taxIncrease);
qDebug() << objectName() << "has cat tax" << catTax();
QCoreApplication::quit();
});
emit hasRequest(req);
}
};
class Processor : public QObject {
Q_OBJECT
public:
Q_SLOT void process(RequestPtr req) {
QThread::msleep(50); // Pretend to do some work.
req->taxIncrease --; // Figure we don't need so many cats after all...
emit req->done(req);
emit done(req);
}
Q_SIGNAL void done(RequestPtr);
};
struct Thread : public QThread { ~Thread() { quit(); wait(); } };
int main(int argc, char ** argv) {
struct C { ~C() { Q_ASSERT(Request::m_count == 0); } } check;
QCoreApplication app(argc, argv);
qRegisterMetaType<RequestPtr>();
Processor processor;
Thread thread;
processor.moveToThread(&thread);
thread.start();
Requester requester1;
requester1.setObjectName("requester1");
QObject::connect(&requester1, &Requester::hasRequest, &processor, &Processor::process);
requester1.sendNewRequest();
{
Requester requester2;
requester2.setObjectName("requester2");
QObject::connect(&requester2, &Requester::hasRequest, &processor, &Processor::process);
requester2.sendNewRequest();
} // requester2 is destructed here
return app.exec();
}
#include "main.moc"
It is impossible to check is that pointer still valid. So, the only safe way here is to inform receiving part about deleting of that QObject
(and in multithreading case: before accessing to object you need to check and block it to be sure, that the object will not be deleted in another thread right after check). The reason of it is simple:
Also it is not safe to just send a signal about deleting of object in multithreading case (or to use QObject::destroyed
as you suggested). Why? Because it is possible, that things happens in this order:
So, in case of only one thread you need QPointer
. Else you need something like QSharedPointer
or QWeakPointer
(both of them are thread-safe) - see answer of Kuba Ober.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With