From 74422bbf0256861899b73de908b45b1a8c8f1aae Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Mon, 10 Jul 2023 17:45:17 +0300 Subject: [PATCH] QTimerInfoList: refactor unregisterTimers() - Use removeIf(), easier to reason about than removing from the list during iteration - Return true only if any timers were actually unregistered - Assert in QObject::event() that if the list returned by eventDispatcher->registeredTimers() isn't empty, then eventDispatcher->unregisteredTimers() returns true Change-Id: I739a3865b7524a7aab3ff0227e6a060ed98d6191 Reviewed-by: Thiago Macieira --- src/corelib/kernel/qobject.cpp | 5 +++- src/corelib/kernel/qtimerinfo_unix.cpp | 32 ++++++++++++++------------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index e53054b6d7..8a31633f38 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -1415,8 +1415,11 @@ bool QObject::event(QEvent *e) if (eventDispatcher) { QList timers = eventDispatcher->registeredTimers(this); if (!timers.isEmpty()) { + const bool res = eventDispatcher->unregisterTimers(this); // do not to release our timer ids back to the pool (since the timer ids are moving to a new thread). - eventDispatcher->unregisterTimers(this); + Q_ASSERT_X(res, Q_FUNC_INFO, + "QAbstractEventDispatcher::unregisterTimers() returned false," + " but there are timers associated with this object."); QMetaObject::invokeMethod(this, "_q_reregisterTimers", Qt::QueuedConnection, Q_ARG(void*, (new QList(timers)))); } diff --git a/src/corelib/kernel/qtimerinfo_unix.cpp b/src/corelib/kernel/qtimerinfo_unix.cpp index bcec650987..3a823cf53e 100644 --- a/src/corelib/kernel/qtimerinfo_unix.cpp +++ b/src/corelib/kernel/qtimerinfo_unix.cpp @@ -353,21 +353,23 @@ bool QTimerInfoList::unregisterTimers(QObject *object) { if (isEmpty()) return false; - for (int i = 0; i < size(); ++i) { - QTimerInfo *t = at(i); - if (t->obj == object) { - // object found - removeAt(i); - if (t == firstTimerInfo) - firstTimerInfo = nullptr; - if (t->activateRef) - *(t->activateRef) = nullptr; - delete t; - // move back one so that we don't skip the new current item - --i; - } - } - return true; + + auto associatedWith = [this](QObject *o) { + return [this, o](auto &t) { + if (t->obj == o) { + if (t == firstTimerInfo) + firstTimerInfo = nullptr; + if (t->activateRef) + *(t->activateRef) = nullptr; + delete t; + return true; + } + return false; + }; + }; + + qsizetype count = removeIf(associatedWith(object)); + return count > 0; } QList QTimerInfoList::registeredTimers(QObject *object) const