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>

4 files changed, 46 insertions, 43 deletions

diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp
index b53f8eafba..3acf9419df 100644
--- a/src/corelib/kernel/qcoreapplication.cpp
+++ b/src/corelib/kernel/qcoreapplication.cpp
@@ -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);
diff --git a/src/corelib/kernel/qcoreevent.cpp b/src/corelib/kernel/qcoreevent.cpp
index 811a3f7599..b4b6ca3c37 100644
--- a/src/corelib/kernel/qcoreevent.cpp
+++ b/src/corelib/kernel/qcoreevent.cpp
@@ -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)
diff --git a/src/corelib/kernel/qcoreevent_p.h b/src/corelib/kernel/qcoreevent_p.h
index edadc01374..2bc85a5e67 100644
--- a/src/corelib/kernel/qcoreevent_p.h
+++ b/src/corelib/kernel/qcoreevent_p.h
@@ -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
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 7154664a0b..55a3bd5308 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -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.
+
+    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;
 
-        d->deleteLaterCalled = true;
+        // 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));
 }
 
 /*!