Fix closure timing hole.

When a closure involves an ObjectHelper, a connection is made from the
receiver's destroyed signal and the helper object's deleteLater slot. Since
the signal between the sender and the helper object isn't disconnected until
either object is actually destroyed, this leaves a hole where the helper
holds a pointer to an invalid receiver object, but is still able to receive
the signal connected to its Invoke slot.

Instead of connecting the destroyed signal to deleteLater, connect it to a new
TearDown slot that immediately disconnects the signal then calls deleteLater.
This commit is contained in:
Jim Broadus 2020-01-11 00:10:53 -08:00
parent a97080a809
commit dbe67bf32b
2 changed files with 18 additions and 4 deletions

View File

@ -37,8 +37,15 @@ ObjectHelper* ClosureBase::helper() const { return helper_; }
ObjectHelper::ObjectHelper(QObject* sender, const char* signal,
ClosureBase* closure)
: closure_(closure) {
connect(sender, signal, SLOT(Invoked()));
connect(sender, SIGNAL(destroyed()), SLOT(deleteLater()));
connection_ = connect(sender, signal, SLOT(Invoked()));
connect(sender, SIGNAL(destroyed()), SLOT(TearDown()));
}
void ObjectHelper::TearDown() {
// For the case that the receiver has been destroyed, disconnect the signal
// so that Invoke isn't called.
disconnect(connection_);
deleteLater();
}
void ObjectHelper::Invoked() {

View File

@ -58,10 +58,14 @@ class ObjectHelper : public QObject {
public:
ObjectHelper(QObject* parent, const char* signal, ClosureBase* closure);
public slots:
void TearDown();
private slots:
void Invoked();
private:
QMetaObject::Connection connection_;
std::unique_ptr<ClosureBase> closure_;
Q_DISABLE_COPY(ObjectHelper)
};
@ -97,8 +101,11 @@ class Closure : public ClosureBase {
const int index = meta_receiver->indexOfSlot(normalised_slot.constData());
Q_ASSERT(index != -1);
slot_ = meta_receiver->method(index);
QObject::connect(receiver_, SIGNAL(destroyed()), helper_,
SLOT(deleteLater()));
// Use a direct connection to insure that this is called immediately when
// the sender is destroyed. This will run on the signal thread, so TearDown
// must be thread safe.
QObject::connect(receiver_, SIGNAL(destroyed()), helper_, SLOT(TearDown()),
Qt::DirectConnection);
}
virtual void Invoke() { function_(); }