From 78acaf4fb6acfc51f8cc823e381e4cf593a715b7 Mon Sep 17 00:00:00 2001 From: Mikolaj Boc Date: Mon, 8 May 2023 08:21:49 +0200 Subject: [PATCH] Fix sending deferred delete events when posted before outermost loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QDeferredDeleteEvent has the loopLevel field, which is a sum of scope and loop levels found at posting. In sendPostedEvents however, it is impossible to only use this information to find delete events posted before the outermost loop (which should be handled by any loop) based solely on this information, as the scope level essentialy removes the information on loop level. Break the loopLevel in two, storing both loop and scope levels in QDeferredDeleteEvent, so that we can check whether an event was posted before the outermost event loop (for which we need to compare only the loop level). QDeferredDeleteEvent was also made private as it should - it is an implementation detail that wasn't hidden properly. Change-Id: I0a607a0bd3a2deb5024acad67f740dbf4338574c Reviewed-by: Morten Johan Sørvig Reviewed-by: Mikołaj Boc --- src/corelib/CMakeLists.txt | 2 +- src/corelib/kernel/qcoreapplication.cpp | 18 ++++++--- src/corelib/kernel/qcoreevent.cpp | 10 +---- src/corelib/kernel/qcoreevent.h | 12 ------ src/corelib/kernel/qcoreevent_p.h | 40 +++++++++++++++++++ src/corelib/kernel/qobject.cpp | 1 + .../qcoreapplication/tst_qcoreapplication.cpp | 29 +++++++++++++- .../qcoreapplication/tst_qcoreapplication.h | 1 + tests/auto/gui/kernel/qevent/tst_qevent.cpp | 1 - .../kernel/qapplication/tst_qapplication.cpp | 5 +++ 10 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 src/corelib/kernel/qcoreevent_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index 164016a208b..a4b654fb5c0 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -152,7 +152,7 @@ qt_internal_add_module(Core kernel/qcoreapplication.cpp kernel/qcoreapplication.h kernel/qcoreapplication_p.h kernel/qcoreapplication_platform.h kernel/qcorecmdlineargs_p.h - kernel/qcoreevent.cpp kernel/qcoreevent.h + kernel/qcoreevent.cpp kernel/qcoreevent.h kernel/qcoreevent_p.h kernel/qdeadlinetimer.cpp kernel/qdeadlinetimer.h kernel/qelapsedtimer.cpp kernel/qelapsedtimer.h kernel/qeventloop.cpp kernel/qeventloop.h kernel/qeventloop_p.h diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 52d0d2ac042..d532875c20f 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -8,6 +8,7 @@ #ifndef QT_NO_QOBJECT #include "qabstracteventdispatcher.h" #include "qcoreevent.h" +#include "qcoreevent_p.h" #include "qeventloop.h" #endif #include "qmetaobject.h" @@ -1660,7 +1661,10 @@ void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority) int scopeLevel = data->scopeLevel; if (scopeLevel == 0 && loopLevel != 0) scopeLevel = 1; - static_cast(event)->level = loopLevel + scopeLevel; + + QDeferredDeleteEvent *deleteEvent = static_cast(event); + deleteEvent->m_loopLevel = loopLevel; + deleteEvent->m_scopeLevel = scopeLevel; } // delete the event on exceptions to protect against memory leaks till the event is @@ -1849,13 +1853,15 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type // events posted by the current event loop; or // 3) if the event was posted before the outermost event loop. - int eventLevel = static_cast(pe.event)->loopLevel(); - int loopLevel = data->loopLevel + data->scopeLevel; + const int eventLoopLevel = static_cast(pe.event)->loopLevel(); + const int eventScopeLevel = static_cast(pe.event)->scopeLevel(); + + const bool postedBeforeOutermostLoop = eventLoopLevel == 0; const bool allowDeferredDelete = - (eventLevel > loopLevel - || (!eventLevel && loopLevel > 0) + (eventLoopLevel + eventScopeLevel > data->loopLevel + data->scopeLevel + || (postedBeforeOutermostLoop && data->loopLevel > 0) || (event_type == QEvent::DeferredDelete - && eventLevel == loopLevel)); + && eventLoopLevel + eventScopeLevel == data->loopLevel + data->scopeLevel)); if (!allowDeferredDelete) { // cannot send deferred delete if (!event_type && !receiver) { diff --git a/src/corelib/kernel/qcoreevent.cpp b/src/corelib/kernel/qcoreevent.cpp index b01f1c2a699..46398872f7e 100644 --- a/src/corelib/kernel/qcoreevent.cpp +++ b/src/corelib/kernel/qcoreevent.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qcoreevent.h" +#include "qcoreevent_p.h" #include "qcoreapplication.h" #include "qcoreapplication_p.h" @@ -637,19 +638,10 @@ Q_IMPL_EVENT_COMMON(QDynamicPropertyChangeEvent) */ QDeferredDeleteEvent::QDeferredDeleteEvent() : QEvent(QEvent::DeferredDelete) - , level(0) { } Q_IMPL_EVENT_COMMON(QDeferredDeleteEvent) -/*! \fn int QDeferredDeleteEvent::loopLevel() const - - Returns the loop-level in which the event was posted. The - loop-level is set by QCoreApplication::postEvent(). - - \sa QObject::deleteLater() -*/ - QT_END_NAMESPACE #include "moc_qcoreevent.cpp" diff --git a/src/corelib/kernel/qcoreevent.h b/src/corelib/kernel/qcoreevent.h index 0c83919bc50..3acaa62a45d 100644 --- a/src/corelib/kernel/qcoreevent.h +++ b/src/corelib/kernel/qcoreevent.h @@ -395,18 +395,6 @@ private: QByteArray n; }; -class Q_CORE_EXPORT QDeferredDeleteEvent : public QEvent -{ - Q_DECL_EVENT_COMMON(QDeferredDeleteEvent) -public: - explicit QDeferredDeleteEvent(); - int loopLevel() const { return level; } - -private: - int level; - friend class QCoreApplication; -}; - QT_END_NAMESPACE #endif // QCOREEVENT_H diff --git a/src/corelib/kernel/qcoreevent_p.h b/src/corelib/kernel/qcoreevent_p.h new file mode 100644 index 00000000000..edadc013741 --- /dev/null +++ b/src/corelib/kernel/qcoreevent_p.h @@ -0,0 +1,40 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only + +#ifndef QCOREEVENT_P_H +#define QCOREEVENT_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#include "QtCore/qcoreevent.h" + +QT_BEGIN_NAMESPACE + +class QCoreApplication; + +class Q_AUTOTEST_EXPORT QDeferredDeleteEvent : public QEvent +{ + Q_DECL_EVENT_COMMON(QDeferredDeleteEvent) +public: + explicit QDeferredDeleteEvent(); + 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 + +#endif // QCOREEVENT_P_H diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index 2fc3d768981..873d3daeca0 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -12,6 +12,7 @@ #include "qabstracteventdispatcher_p.h" #include "qcoreapplication.h" #include "qcoreapplication_p.h" +#include "qcoreevent_p.h" #include "qloggingcategory.h" #include "qvariant.h" #include "qmetaobject.h" diff --git a/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.cpp b/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.cpp index edba67bf46e..73fe06a83f7 100644 --- a/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.cpp +++ b/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.cpp @@ -9,6 +9,7 @@ #include // for qGlobalPostedEventsCount() #include +#include #include #include @@ -24,9 +25,12 @@ class EventSpy : public QObject public: QList recordedEvents; - bool eventFilter(QObject *, QEvent *event) override + std::function eventCallback; + bool eventFilter(QObject *target, QEvent *event) override { recordedEvents.append(event->type()); + if (eventCallback) + eventCallback(target, event); return false; } }; @@ -1081,6 +1085,29 @@ static void createQObjectOnDestruction() } Q_DESTRUCTOR_FUNCTION(createQObjectOnDestruction) +void tst_QCoreApplication::testDeleteLaterFromBeforeOutermostEventLoop() +{ + int argc = 0; + QCoreApplication app(argc, nullptr); + + EventSpy *spy = new EventSpy(); + QPointer spyPointer = spy; + + app.installEventFilter(spy); + spy->eventCallback = [spy](QObject *, QEvent *event) { + if (event->type() == QEvent::User + 1) + spy->deleteLater(); + }; + + QCoreApplication::postEvent(&app, new QEvent(QEvent::Type(QEvent::User + 1))); + QCoreApplication::processEvents(); + + QEventLoop loop; + QTimer::singleShot(0, &loop, &QEventLoop::quit); + loop.exec(); + QVERIFY(!spyPointer); +} + #ifndef QT_QGUIAPPLICATIONTEST QTEST_APPLESS_MAIN(tst_QCoreApplication) #endif diff --git a/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.h b/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.h index 91d9324db4b..8271c0cbb27 100644 --- a/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.h +++ b/tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.h @@ -45,6 +45,7 @@ private slots: void addRemoveLibPaths(); #endif void theMainThread(); + void testDeleteLaterFromBeforeOutermostEventLoop(); }; #endif // TST_QCOREAPPLICATION_H diff --git a/tests/auto/gui/kernel/qevent/tst_qevent.cpp b/tests/auto/gui/kernel/qevent/tst_qevent.cpp index f1ffb8c35ee..56d483107be 100644 --- a/tests/auto/gui/kernel/qevent/tst_qevent.cpp +++ b/tests/auto/gui/kernel/qevent/tst_qevent.cpp @@ -14,7 +14,6 @@ X(QTimerEvent, (42)) \ X(QChildEvent, (QEvent::ChildAdded, nullptr)) \ X(QDynamicPropertyChangeEvent, ("size")) \ - X(QDeferredDeleteEvent, ()) \ /* qfutureinterface_p.h */ \ X(QFutureCallOutEvent, ()) \ /* end */ diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp index 3ed9743838c..fe98b4a9c11 100644 --- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp +++ b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp @@ -19,6 +19,7 @@ #if QT_CONFIG(process) # include #endif +#include #include #include @@ -94,7 +95,9 @@ private slots: void libraryPaths_qt_plugin_path_2(); #endif +#ifdef QT_BUILD_INTERNAL void sendPostedEvents(); +#endif // ifdef QT_BUILD_INTERNAL void thread(); void desktopSettingsAware(); @@ -1126,6 +1129,7 @@ void tst_QApplication::libraryPaths_qt_plugin_path_2() } #endif +#ifdef QT_BUILD_INTERNAL class SendPostedEventsTester : public QObject { Q_OBJECT @@ -1171,6 +1175,7 @@ void tst_QApplication::sendPostedEvents() (void) QCoreApplication::exec(); QVERIFY(p.isNull()); } +#endif void tst_QApplication::thread() {