Move QDeferredDeleteEvent loop level/scope handling into deleteLater()

We have all the information we need when deleteLater() is called,
so there's no point in deferring it until the event is posted,
which requires friended access into the QDeferredDeleteEvent's
members.

Moving the code focuses QCoreApplication::postEvent() on its
primary task, posting of the event (adding to the event list,
waking up the event dispatcher).

It's also easier to reason about how the action of deleteLater
on an object relates to the event loop and scope level when
the information is resolved up front.

Task-number: QTBUG-120124
Change-Id: If38f601ff653111763004b98915b01ffe8ddc837
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Tor Arne Vestbø 2023-12-10 18:12:25 +01:00
parent e02dc31fbf
commit 46588fbb51
4 changed files with 46 additions and 43 deletions

View File

@ -1677,31 +1677,6 @@ void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority)
return;
}
if (event->type() == QEvent::DeferredDelete && data == QThreadData::current()) {
// remember the current running eventloop for DeferredDelete
// events posted in the receiver's thread.
// Events sent by non-Qt event handlers (such as glib) may not
// have the scopeLevel set correctly. The scope level makes sure that
// code like this:
// foo->deleteLater();
// qApp->processEvents(); // without passing QEvent::DeferredDelete
// will not cause "foo" to be deleted before returning to the event loop.
// If the scope level is 0 while loopLevel != 0, we are called from a
// non-conformant code path, and our best guess is that the scope level
// should be 1. (Loop level 0 is special: it means that no event loops
// are running.)
int loopLevel = data->loopLevel;
int scopeLevel = data->scopeLevel;
if (scopeLevel == 0 && loopLevel != 0)
scopeLevel = 1;
QDeferredDeleteEvent *deleteEvent = static_cast<QDeferredDeleteEvent *>(event);
deleteEvent->m_loopLevel = loopLevel;
deleteEvent->m_scopeLevel = scopeLevel;
}
// delete the event on exceptions to protect against memory leaks till the event is
// properly owned in the postEventList
std::unique_ptr<QEvent> eventDeleter(event);

View File

@ -642,10 +642,10 @@ Q_IMPL_EVENT_COMMON(QDynamicPropertyChangeEvent)
*/
/*!
Constructs a deferred delete event with an initial loopLevel() of zero.
Constructs a deferred delete event with the given loop and scope level.
*/
QDeferredDeleteEvent::QDeferredDeleteEvent()
: QEvent(QEvent::DeferredDelete)
QDeferredDeleteEvent::QDeferredDeleteEvent(int loopLevel, int scopeLevel)
: QEvent(QEvent::DeferredDelete), m_loopLevel(loopLevel), m_scopeLevel(scopeLevel)
{ }
Q_IMPL_EVENT_COMMON(QDeferredDeleteEvent)

View File

@ -25,14 +25,13 @@ class Q_AUTOTEST_EXPORT QDeferredDeleteEvent : public QEvent
{
Q_DECL_EVENT_COMMON(QDeferredDeleteEvent)
public:
explicit QDeferredDeleteEvent();
explicit QDeferredDeleteEvent(int loopLevel, int scopeLevel);
int loopLevel() const { return m_loopLevel; }
int scopeLevel() const { return m_scopeLevel; }
private:
int m_loopLevel = 0;
int m_scopeLevel = 0;
friend class QCoreApplication;
};
QT_END_NAMESPACE

View File

@ -2437,24 +2437,53 @@ void QObject::deleteLater()
qWarning("You are deferring the delete of QCoreApplication, this may not work as expected.");
#endif
{
// De-bounce QDeferredDeleteEvents. Use the post event list mutex
// to guard access to deleteLaterCalled, so we don't need a separate
// mutex in QObjectData.
auto locker = QCoreApplicationPrivate::lockThreadPostEventList(this);
// FIXME: The deleteLaterCalled flag is part of a bit field,
// so we likely have data races here, even with the mutex above,
// as long as we're not guarding every access to the bit field.
// De-bounce QDeferredDeleteEvents. Use the post event list mutex
// to guard access to deleteLaterCalled, so we don't need a separate
// mutex in QObjectData.
auto eventListLocker = QCoreApplicationPrivate::lockThreadPostEventList(this);
if (!eventListLocker.threadData)
return;
Q_D(QObject);
if (d->deleteLaterCalled)
return;
// FIXME: The deleteLaterCalled flag is part of a bit field,
// so we likely have data races here, even with the mutex above,
// as long as we're not guarding every access to the bit field.
d->deleteLaterCalled = true;
Q_D(QObject);
if (d->deleteLaterCalled)
return;
d->deleteLaterCalled = true;
int loopLevel = 0;
int scopeLevel = 0;
auto *objectThreadData = eventListLocker.threadData;
if (objectThreadData == QThreadData::current()) {
// Remember the current running eventloop for deleteLater
// calls in the object's own thread.
// Events sent by non-Qt event handlers (such as glib) may not
// have the scopeLevel set correctly. The scope level makes sure that
// code like this:
// foo->deleteLater();
// qApp->processEvents(); // without passing QEvent::DeferredDelete
// will not cause "foo" to be deleted before returning to the event loop.
loopLevel = objectThreadData->loopLevel;
scopeLevel = objectThreadData->scopeLevel;
// If the scope level is 0 while loopLevel != 0, we are called from a
// non-conformant code path, and our best guess is that the scope level
// should be 1. (Loop level 0 is special: it means that no event loops
// are running.)
if (scopeLevel == 0 && loopLevel != 0)
scopeLevel = 1;
}
QCoreApplication::postEvent(this, new QDeferredDeleteEvent());
eventListLocker.unlock();
QCoreApplication::postEvent(this,
new QDeferredDeleteEvent(loopLevel, scopeLevel));
}
/*!