1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QObject: Fix memory leak in queued_activate

Browse Source 
queued_activate adds a reference to the slot object. It also attempts to
deref it again, but that did not work correctly so far. We could end up
with
T1                              |             T2
queued_activate                 |
  checks isSlotObject           |
    adds ref                    |
  locker.unlock()               |
                                | QObject::~QObject
                                |  //In disconnect all senders loop
                                |  sets isSlotObject to false
                                |  derefs slotObj, but not deleted
  checks isSlotObject           |
  (no deref because it's null)  |

To solve this issue and others caused by early returns, we now use a
RAII helper, which always takes care of calling destroyIfLastRef if the
ref count has been incremented.

Change-Id: I9c011cdb8faa5f344d7e70f024fc13f407e39ccf
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
This commit is contained in:
Fabian Kosmale 2021-06-11 09:43:37 +02:00
parent 1d05dcb3ec
commit 44fa80cbd4
1 changed files with 34 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
src/corelib/kernel/qobject.cpp
View File

@ -3647,6 +3647,37 @@ void QMetaObject::connectSlotsByName(QObject *o)
} }
} }
/*!
\internal
A small RAII helper for QSlotObjectBase.
Calls ref on construction and destroyLastRef in its dtor.
Allows construction from a nullptr in which case it does nothing.
*/
struct SlotObjectGuard {
SlotObjectGuard() = default;
// move would be fine, but we do not need it currently
Q_DISABLE_COPY_MOVE(SlotObjectGuard)
explicit SlotObjectGuard(QtPrivate::QSlotObjectBase *slotObject)
: m_slotObject(slotObject)
{
if (m_slotObject)
m_slotObject->ref();
}
QtPrivate::QSlotObjectBase const *operator->() const
{ return m_slotObject; }
QtPrivate::QSlotObjectBase *operator->()
{ return m_slotObject; }
~SlotObjectGuard() {
if (m_slotObject)
m_slotObject->destroyIfLastRef();
}
private:
QtPrivate::QSlotObjectBase *m_slotObject = nullptr;
};
/*! /*!
\internal \internal
@ -3678,8 +3709,8 @@ static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connect
// the connection has been disconnected before we got the lock // the connection has been disconnected before we got the lock
return; return;
} }
if (c->isSlotObject)
c->slotObj->ref(); SlotObjectGuard slotObjectGuard { c->isSlotObject ? c->slotObj : nullptr };
locker.unlock(); locker.unlock();
QMetaCallEvent *ev = c->isSlotObject ? QMetaCallEvent *ev = c->isSlotObject ?
@ -3706,8 +3737,6 @@ static void queued_activate(QObject *sender, int signal, QObjectPrivate::Connect
} }
locker.relock(); locker.relock();
if (c->isSlotObject)
c->slotObj->destroyIfLastRef();
if (!c->isSingleShot && !c->receiver.loadRelaxed()) { if (!c->isSingleShot && !c->receiver.loadRelaxed()) {
// the connection has been disconnected while we were unlocked // the connection has been disconnected while we were unlocked
locker.unlock(); locker.unlock();
@ -3835,14 +3864,7 @@ void doActivate(QObject *sender, int signal_index, void **argv)
QObjectPrivate::Sender senderData(receiverInSameThread ? receiver : nullptr, sender, signal_index); QObjectPrivate::Sender senderData(receiverInSameThread ? receiver : nullptr, sender, signal_index);
if (c->isSlotObject) { if (c->isSlotObject) {
c->slotObj->ref(); SlotObjectGuard obj{c->slotObj};
struct Deleter {
void operator()(QtPrivate::QSlotObjectBase *slot) const {
if (slot) slot->destroyIfLastRef();
}
};
const std::unique_ptr<QtPrivate::QSlotObjectBase, Deleter> obj{c->slotObj};
{ {
Q_TRACE_SCOPE(QMetaObject_activate_slot_functor, obj.get()); Q_TRACE_SCOPE(QMetaObject_activate_slot_functor, obj.get());