QTimerInfoList: don't inherit from a container in an exported class

Instead use composition by using a container member; inheriting from
containers has a couple of issues:
- containers (STL and Qt) don't have virtual destructors, which means
  that deleting via a pointer to the base-class would be troublesome
- as it turns out, inheriting from containers, QList in this case,
  in an exported derived-class could lead to binary-incompatibility
  issues (see linked bug report)

Drive-by change:
- group all private members in one place
- make timerInsert() private, nothing should use it from outside the
  class anyway

Change-Id: I69843835d8c854fa4b13e8b4ba3fb69f1484f6a6
Fixes: QTBUG-111959
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ahmad Samir 2023-02-27 13:11:35 +02:00
parent 74422bbf02
commit 619b03401d
6 changed files with 44 additions and 34 deletions

View File

@ -195,7 +195,7 @@ void QEventDispatcherCoreFoundation::startingUp()
QEventDispatcherCoreFoundation::~QEventDispatcherCoreFoundation()
{
invalidateTimer();
qDeleteAll(m_timerInfoList);
m_timerInfoList.clearTimers();
m_cfSocketNotifier.removeSocketNotifiers();
}

View File

@ -335,7 +335,7 @@ QEventDispatcherGlib::~QEventDispatcherGlib()
Q_D(QEventDispatcherGlib);
// destroy all timer sources
qDeleteAll(d->timerSource->timerList);
d->timerSource->timerList.clearTimers();
d->timerSource->timerList.~QTimerInfoList();
g_source_destroy(&d->timerSource->source);
g_source_unref(&d->timerSource->source);

View File

@ -195,7 +195,7 @@ QEventDispatcherUNIXPrivate::QEventDispatcherUNIXPrivate()
QEventDispatcherUNIXPrivate::~QEventDispatcherUNIXPrivate()
{
// cleanup timers
qDeleteAll(timerList);
timerList.clearTimers();
}
void QEventDispatcherUNIXPrivate::setSocketNotifierPending(QSocketNotifier *notifier)

View File

@ -41,9 +41,9 @@ steady_clock::time_point QTimerInfoList::updateCurrentTime()
*/
bool QTimerInfoList::hasPendingTimers()
{
if (isEmpty())
if (timers.isEmpty())
return false;
return updateCurrentTime() < constFirst()->timeout;
return updateCurrentTime() < timers.at(0)->timeout;
}
static bool byTimeout(const QTimerInfo *a, const QTimerInfo *b)
@ -54,8 +54,8 @@ static bool byTimeout(const QTimerInfo *a, const QTimerInfo *b)
*/
void QTimerInfoList::timerInsert(QTimerInfo *ti)
{
insert(std::upper_bound(cbegin(), cend(), ti, byTimeout),
ti);
timers.insert(std::upper_bound(timers.cbegin(), timers.cend(), ti, byTimeout),
ti);
}
static constexpr milliseconds roundToMillisecond(nanoseconds val)
@ -236,12 +236,11 @@ bool QTimerInfoList::timerWait(timespec &tm)
auto isWaiting = [](QTimerInfo *tinfo) { return !tinfo->activateRef; };
// Find first waiting timer not already active
auto it = std::find_if(cbegin(), cend(), isWaiting);
if (it == cend())
auto it = std::find_if(timers.cbegin(), timers.cend(), isWaiting);
if (it == timers.cend())
return false;
QTimerInfo *t = *it;
nanoseconds timeToWait = t->timeout - now;
nanoseconds timeToWait = (*it)->timeout - now;
if (timeToWait > 0ns)
tm = durationToTimespec(roundToMillisecond(timeToWait));
else
@ -265,7 +264,7 @@ milliseconds QTimerInfoList::remainingDuration(int timerId)
const steady_clock::time_point now = updateCurrentTime();
auto it = findTimerById(timerId);
if (it == cend()) {
if (it == timers.cend()) {
#ifndef QT_NO_DEBUG
qWarning("QTimerInfoList::timerRemainingTime: timer id %i not found", timerId);
#endif
@ -335,7 +334,7 @@ void QTimerInfoList::registerTimer(int timerId, milliseconds interval,
bool QTimerInfoList::unregisterTimer(int timerId)
{
auto it = findTimerById(timerId);
if (it == cend())
if (it == timers.cend())
return false; // id not found
// set timer inactive
@ -345,13 +344,13 @@ bool QTimerInfoList::unregisterTimer(int timerId)
if (t->activateRef)
*(t->activateRef) = nullptr;
delete t;
erase(it);
timers.erase(it);
return true;
}
bool QTimerInfoList::unregisterTimers(QObject *object)
{
if (isEmpty())
if (timers.isEmpty())
return false;
auto associatedWith = [this](QObject *o) {
@ -368,14 +367,14 @@ bool QTimerInfoList::unregisterTimers(QObject *object)
};
};
qsizetype count = removeIf(associatedWith(object));
qsizetype count = timers.removeIf(associatedWith(object));
return count > 0;
}
QList<QAbstractEventDispatcher::TimerInfo> QTimerInfoList::registeredTimers(QObject *object) const
{
QList<QAbstractEventDispatcher::TimerInfo> list;
for (const QTimerInfo *const t : std::as_const(*this)) {
for (const auto &t : timers) {
if (t->obj == object)
list.emplaceBack(t->id, t->interval.count(), t->timerType);
}
@ -387,7 +386,7 @@ QList<QAbstractEventDispatcher::TimerInfo> QTimerInfoList::registeredTimers(QObj
*/
int QTimerInfoList::activateTimers()
{
if (qt_disable_lowpriority_timers || isEmpty())
if (qt_disable_lowpriority_timers || timers.isEmpty())
return 0; // nothing to do
firstTimerInfo = nullptr;
@ -397,16 +396,16 @@ int QTimerInfoList::activateTimers()
// Find out how many timer have expired
auto stillActive = [&now](const QTimerInfo *t) { return now < t->timeout; };
// Find first one still active (list is sorted by timeout)
auto it = std::find_if(cbegin(), cend(), stillActive);
auto maxCount = it - cbegin();
auto it = std::find_if(timers.cbegin(), timers.cend(), stillActive);
auto maxCount = it - timers.cbegin();
int n_act = 0;
//fire the timers.
while (maxCount--) {
if (isEmpty())
if (timers.isEmpty())
break;
QTimerInfo *currentTimerInfo = constFirst();
QTimerInfo *currentTimerInfo = timers.constFirst();
if (now < currentTimerInfo->timeout)
break; // no timer has expired
@ -422,12 +421,12 @@ int QTimerInfoList::activateTimers()
// determine next timeout time
calculateNextTimeout(currentTimerInfo, now);
if (size() > 1) {
if (timers.size() > 1) {
// Find where "currentTimerInfo" should be in the list so as
// to keep the list ordered by timeout
auto afterCurrentIt = begin() + 1;
auto iter = std::upper_bound(afterCurrentIt, end(), currentTimerInfo, byTimeout);
currentTimerInfo = *std::rotate(begin(), afterCurrentIt, iter);
auto afterCurrentIt = timers.begin() + 1;
auto iter = std::upper_bound(afterCurrentIt, timers.end(), currentTimerInfo, byTimeout);
currentTimerInfo = *std::rotate(timers.begin(), afterCurrentIt, iter);
}
if (currentTimerInfo->interval > 0ms)

View File

@ -34,11 +34,8 @@ struct QTimerInfo {
QTimerInfo **activateRef; // - ref from activateTimers
};
class Q_CORE_EXPORT QTimerInfoList : public QList<QTimerInfo*>
class Q_CORE_EXPORT QTimerInfoList
{
// state variables used by activateTimers()
QTimerInfo *firstTimerInfo = nullptr;
public:
QTimerInfoList();
@ -60,14 +57,28 @@ public:
int activateTimers();
bool hasPendingTimers();
QList::const_iterator findTimerById(int timerId) const
void clearTimers()
{
auto matchesId = [timerId](const QTimerInfo *t) { return t->id == timerId; };
return std::find_if(cbegin(), cend(), matchesId);
qDeleteAll(timers);
timers.clear();
}
bool isEmpty() const { return timers.empty(); }
qsizetype size() const { return timers.size(); }
auto findTimerById(int timerId)
{
auto matchesId = [timerId](const auto &t) { return t->id == timerId; };
return std::find_if(timers.cbegin(), timers.cend(), matchesId);
}
private:
std::chrono::steady_clock::time_point updateCurrentTime();
// state variables used by activateTimers()
QTimerInfo *firstTimerInfo = nullptr;
QList<QTimerInfo *> timers;
};
QT_END_NAMESPACE

View File

@ -957,7 +957,7 @@ QCocoaEventDispatcher::~QCocoaEventDispatcher()
{
Q_D(QCocoaEventDispatcher);
qDeleteAll(d->timerInfoList);
d->timerInfoList.clearTimers();
d->maybeStopCFRunLoopTimer();
CFRunLoopRemoveSource(mainRunLoop(), d->activateTimersSourceRef, kCFRunLoopCommonModes);
CFRelease(d->activateTimersSourceRef);