From 5c94e1195a6a3dc9181dc86e58cd79833f21364f Mon Sep 17 00:00:00 2001 From: Lars Toenning Date: Mon, 9 Jun 2025 16:15:40 +0200 Subject: [PATCH] [skip ci] refactor: Use composition for update timer The timer is not required by all classes and the deep inheritance makes it hard to see where it is actually used. Use composition instead --- src/core/afv/clients/afvclient.cpp | 2 - src/core/airspaceanalyzer.cpp | 14 +++--- src/core/airspaceanalyzer.h | 2 + src/core/context/contextsimulatorimpl.cpp | 3 +- src/core/db/backgrounddataupdater.cpp | 5 +- src/core/db/backgrounddataupdater.h | 2 + src/core/threadedreaderperiodic.cpp | 27 ++++------ src/core/threadedreaderperiodic.h | 11 +++-- src/core/vatsim/vatsimdatafilereader.cpp | 3 +- src/core/vatsim/vatsimmetarreader.cpp | 3 +- src/misc/CMakeLists.txt | 2 + src/misc/simulation/backgroundvalidation.cpp | 16 ++++-- src/misc/simulation/backgroundvalidation.h | 7 +++ src/misc/threadedtimer.cpp | 52 ++++++++++++++++++++ src/misc/threadedtimer.h | 42 ++++++++++++++++ src/misc/worker.cpp | 46 ----------------- src/misc/worker.h | 9 ---- 17 files changed, 154 insertions(+), 92 deletions(-) create mode 100644 src/misc/threadedtimer.cpp create mode 100644 src/misc/threadedtimer.h diff --git a/src/core/afv/clients/afvclient.cpp b/src/core/afv/clients/afvclient.cpp index bcdee4fca..b829c4b3f 100644 --- a/src/core/afv/clients/afvclient.cpp +++ b/src/core/afv/clients/afvclient.cpp @@ -52,8 +52,6 @@ namespace swift::core::afv::clients connect(m_connection, &CClientConnection::audioReceived, this, &CAfvClient::audioOutDataAvailable); connect(m_voiceServerTimer, &QTimer::timeout, this, &CAfvClient::onTimerUpdate); - m_updateTimer.stop(); // not used - // deferred init - use swift::misc:: singleShot to call in correct thread, "myself" NOT needed swift::misc::singleShot(1000, this, [=] { this->deferredInit(); }); } diff --git a/src/core/airspaceanalyzer.cpp b/src/core/airspaceanalyzer.cpp index 242677917..bbdec589a 100644 --- a/src/core/airspaceanalyzer.cpp +++ b/src/core/airspaceanalyzer.cpp @@ -27,20 +27,22 @@ using namespace swift::misc::physical_quantities; using namespace swift::misc::simulation; using namespace swift::core::fsd; +static constexpr std::chrono::milliseconds updateInterval { 7500 }; + namespace swift::core { CAirspaceAnalyzer::CAirspaceAnalyzer(IOwnAircraftProvider *ownAircraftProvider, CFSDClient *fsdClient, CAirspaceMonitor *airspaceMonitorParent) : CContinuousWorker(airspaceMonitorParent, "CAirspaceAnalyzer"), COwnAircraftAware(ownAircraftProvider), - CRemoteAircraftAware(airspaceMonitorParent) + CRemoteAircraftAware(airspaceMonitorParent), m_updateTimer(airspaceMonitorParent, "CAirspaceAnalyzer") { Q_ASSERT_X(fsdClient, Q_FUNC_INFO, "Network object required to connect"); // all in new thread from here on this->setObjectName(this->getName()); - m_updateTimer.start(7500); + m_updateTimer.startTimer(updateInterval); m_lastWatchdogCallMsSinceEpoch = QDateTime::currentMSecsSinceEpoch(); - bool c = connect(&m_updateTimer, &QTimer::timeout, this, &CAirspaceAnalyzer::onTimeout); + bool c = connect(&m_updateTimer, &CThreadedTimer::timeout, this, &CAirspaceAnalyzer::onTimeout); Q_ASSERT(c); // network connected @@ -134,9 +136,9 @@ namespace swift::core if (newStatus.isDisconnected()) { this->clear(); - m_updateTimer.stop(); + m_updateTimer.stopTimer(); } - else if (newStatus.isConnected()) { m_updateTimer.start(); } + else if (newStatus.isConnected()) { m_updateTimer.startTimer(updateInterval); } } void CAirspaceAnalyzer::onTimeout() @@ -170,7 +172,7 @@ namespace swift::core // this is a trick to not remove everything while debugging const qint64 currentTimeMsEpoch = QDateTime::currentMSecsSinceEpoch(); const qint64 callDiffMs = currentTimeMsEpoch - m_lastWatchdogCallMsSinceEpoch; - const qint64 callThresholdMs = static_cast(m_updateTimer.interval() * 1.5); + const qint64 callThresholdMs = static_cast(updateInterval.count() * 1.5); m_lastWatchdogCallMsSinceEpoch = currentTimeMsEpoch; if (callDiffMs > callThresholdMs) { diff --git a/src/core/airspaceanalyzer.h b/src/core/airspaceanalyzer.h index 4eb810f54..a4b88a2ac 100644 --- a/src/core/airspaceanalyzer.h +++ b/src/core/airspaceanalyzer.h @@ -24,6 +24,7 @@ #include "misc/simulation/airspaceaircraftsnapshot.h" #include "misc/simulation/ownaircraftprovider.h" #include "misc/simulation/remoteaircraftprovider.h" +#include "misc/threadedtimer.h" #include "misc/worker.h" namespace swift::misc::aviation @@ -134,6 +135,7 @@ namespace swift::core qint64 m_lastWatchdogCallMsSinceEpoch; //!< when last called qint64 m_doNotRunAgainBefore = -1; //!< do not run again before, also used to detect debugging std::atomic_bool m_enabledWatchdog { true }; //!< watchdog enabled + misc::CThreadedTimer m_updateTimer; //!< Thread safe timer for update timeout // snapshot swift::misc::simulation::CAirspaceAircraftSnapshot m_latestAircraftSnapshot; diff --git a/src/core/context/contextsimulatorimpl.cpp b/src/core/context/contextsimulatorimpl.cpp index 20177750e..6cb1f6455 100644 --- a/src/core/context/contextsimulatorimpl.cpp +++ b/src/core/context/contextsimulatorimpl.cpp @@ -90,7 +90,8 @@ namespace swift::core::context Qt::QueuedConnection); m_validator->start(QThread::LowestPriority); - m_validator->startUpdating(60); + using namespace std::chrono_literals; + m_validator->startUpdating(60s); } // For validation we need simulator directory and model directory diff --git a/src/core/db/backgrounddataupdater.cpp b/src/core/db/backgrounddataupdater.cpp index 31f47e616..b2c897980 100644 --- a/src/core/db/backgrounddataupdater.cpp +++ b/src/core/db/backgrounddataupdater.cpp @@ -30,9 +30,10 @@ namespace swift::core::db return cats; } - CBackgroundDataUpdater::CBackgroundDataUpdater(QObject *owner) : CContinuousWorker(owner, "Background data updater") + CBackgroundDataUpdater::CBackgroundDataUpdater(QObject *owner) + : CContinuousWorker(owner, "Background data updater"), m_updateTimer(owner, "Background data updater") { - connect(&m_updateTimer, &QTimer::timeout, this, &CBackgroundDataUpdater::doWork); + connect(&m_updateTimer, &misc::CThreadedTimer::timeout, this, &CBackgroundDataUpdater::doWork); if (sApp && sApp->hasWebDataServices()) { connect(sApp->getWebDataServices()->getDatabaseWriter(), &CDatabaseWriter::publishedModelsSimplified, this, diff --git a/src/core/db/backgrounddataupdater.h b/src/core/db/backgrounddataupdater.h index 1581b3df3..e9967a913 100644 --- a/src/core/db/backgrounddataupdater.h +++ b/src/core/db/backgrounddataupdater.h @@ -16,6 +16,7 @@ #include "misc/network/entityflags.h" #include "misc/simulation/data/modelcaches.h" #include "misc/statusmessagelist.h" +#include "misc/threadedtimer.h" #include "misc/worker.h" namespace swift::core::db @@ -47,6 +48,7 @@ namespace swift::core::db std::atomic_bool m_updatePublishedModels { true }; //!< update when models have been updated QMap m_syncedModelsLatestChange; //! timestamp per cache when last synced swift::misc::CStatusMessageList m_messageHistory; + misc::CThreadedTimer m_updateTimer; //!< Thread safe timer for update timeout // set/caches as member as we are in own thread, central instance will not work swift::misc::simulation::data::CModelCaches m_modelCaches { false, this }; diff --git a/src/core/threadedreaderperiodic.cpp b/src/core/threadedreaderperiodic.cpp index 3e802123f..9220b32cf 100644 --- a/src/core/threadedreaderperiodic.cpp +++ b/src/core/threadedreaderperiodic.cpp @@ -5,40 +5,33 @@ namespace swift::core { - CThreadedReaderPeriodic::CThreadedReaderPeriodic(QObject *owner, const QString &name) : CThreadedReader(owner, name) + CThreadedReaderPeriodic::CThreadedReaderPeriodic(QObject *owner, const QString &name) + : CThreadedReader(owner, name), m_updateTimer(owner, name, true) { - connect(&m_updateTimer, &QTimer::timeout, this, &CThreadedReaderPeriodic::doWork); - m_updateTimer.setSingleShot(true); + connect(&m_updateTimer, &misc::CThreadedTimer::timeout, this, &CThreadedReaderPeriodic::doWork); } void CThreadedReaderPeriodic::startReader() { Q_ASSERT_X(hasStarted(), Q_FUNC_INFO, "Thread was not started yet!"); - Q_ASSERT(m_initialTime > 0); - QTimer::singleShot(m_initialTime, this, [=] { this->doWork(); }); + QTimer::singleShot(m_initialTime.load(), this, [=] { this->doWork(); }); } - void CThreadedReaderPeriodic::setInitialAndPeriodicTime(int initialTime, int periodicTime) + void CThreadedReaderPeriodic::setInitialAndPeriodicTime(std::chrono::milliseconds initialTime, + std::chrono::milliseconds periodicTime) { m_initialTime = initialTime; m_periodicTime = periodicTime; - - // if timer is active start with delta time - // remark: will be reset in doWork - if (m_updateTimer.isActive()) - { - const int oldPeriodicTime = m_updateTimer.interval(); - const int delta = m_periodicTime - oldPeriodicTime + m_updateTimer.remainingTime(); - m_updateTimer.start(qMax(delta, 0)); - } } void CThreadedReaderPeriodic::doWork() { if (!doWorkCheck()) { return; } this->doWorkImpl(); - Q_ASSERT(m_periodicTime > 0); - m_updateTimer.start(m_periodicTime); // restart + using namespace std::chrono_literals; + Q_ASSERT(m_periodicTime.load() > 0ms); + + m_updateTimer.startTimer(m_periodicTime); // restart } } // namespace swift::core diff --git a/src/core/threadedreaderperiodic.h b/src/core/threadedreaderperiodic.h index 5a1e57c86..9c9e7c2a5 100644 --- a/src/core/threadedreaderperiodic.h +++ b/src/core/threadedreaderperiodic.h @@ -11,6 +11,7 @@ #include "core/swiftcoreexport.h" #include "core/threadedreader.h" +#include "misc/threadedtimer.h" namespace swift::core { @@ -34,14 +35,18 @@ namespace swift::core virtual void doWorkImpl() = 0; //! Set initial and periodic times - void setInitialAndPeriodicTime(int initialTime, int periodicTime); + //! Changes only apply after the next time the timer restarts + //! \threadsafe + void setInitialAndPeriodicTime(std::chrono::milliseconds initialTime, std::chrono::milliseconds periodicTime); private: //! Trigger doWorkImpl void doWork(); - int m_initialTime = -1; //!< Initial start delay - int m_periodicTime = -1; //!< Periodic time after which the task is repeated + std::atomic m_initialTime = std::chrono::milliseconds(0); //!< Initial start delay + std::atomic m_periodicTime = std::chrono::milliseconds(0); //!< Periodic time after which the task is repeated + + misc::CThreadedTimer m_updateTimer; //!< Update timer }; } // namespace swift::core diff --git a/src/core/vatsim/vatsimdatafilereader.cpp b/src/core/vatsim/vatsimdatafilereader.cpp index 83d3dd346..a65ee3381 100644 --- a/src/core/vatsim/vatsimdatafilereader.cpp +++ b/src/core/vatsim/vatsimdatafilereader.cpp @@ -317,6 +317,7 @@ namespace swift::core::vatsim void CVatsimDataFileReader::reloadSettings() { CReaderSettings s = m_settings.get(); - setInitialAndPeriodicTime(s.getInitialTime().toMs(), s.getPeriodicTime().toMs()); + setInitialAndPeriodicTime(std::chrono::milliseconds(s.getInitialTime().toMs()), + std::chrono::milliseconds(s.getPeriodicTime().toMs())); } } // namespace swift::core::vatsim diff --git a/src/core/vatsim/vatsimmetarreader.cpp b/src/core/vatsim/vatsimmetarreader.cpp index 8a094c4f0..e15875930 100644 --- a/src/core/vatsim/vatsimmetarreader.cpp +++ b/src/core/vatsim/vatsimmetarreader.cpp @@ -131,6 +131,7 @@ namespace swift::core::vatsim void CVatsimMetarReader::reloadSettings() { const CReaderSettings s = m_settings.get(); - this->setInitialAndPeriodicTime(s.getInitialTime().toMs(), s.getPeriodicTime().toMs()); + this->setInitialAndPeriodicTime(std::chrono::milliseconds(s.getInitialTime().toMs()), + std::chrono::milliseconds(s.getPeriodicTime().toMs())); } } // namespace swift::core::vatsim diff --git a/src/misc/CMakeLists.txt b/src/misc/CMakeLists.txt index 359c9e4f4..40bd0d4c2 100644 --- a/src/misc/CMakeLists.txt +++ b/src/misc/CMakeLists.txt @@ -319,6 +319,8 @@ add_library(misc SHARED swiftdirectories.cpp swiftdirectories.h swiftmiscexport.h + threadedtimer.cpp + threadedtimer.h threadutils.cpp threadutils.h timestampbased.cpp diff --git a/src/misc/simulation/backgroundvalidation.cpp b/src/misc/simulation/backgroundvalidation.cpp index 1979c858d..71658a10c 100644 --- a/src/misc/simulation/backgroundvalidation.cpp +++ b/src/misc/simulation/backgroundvalidation.cpp @@ -22,9 +22,10 @@ namespace swift::misc::simulation return cats; } - CBackgroundValidation::CBackgroundValidation(QObject *owner) : CContinuousWorker(owner, "Background validation") + CBackgroundValidation::CBackgroundValidation(QObject *owner) + : CContinuousWorker(owner, "Background validation"), m_updateTimer(owner, "Background validation") { - connect(&m_updateTimer, &QTimer::timeout, this, &CBackgroundValidation::doWork); + connect(&m_updateTimer, &CThreadedTimer::timeout, this, &CBackgroundValidation::doWork); } void CBackgroundValidation::setCurrentSimulator(const CSimulatorInfo &simulator, const QString &simDirectory, @@ -94,10 +95,17 @@ namespace swift::misc::simulation return true; } + void CBackgroundValidation::startUpdating(std::chrono::milliseconds ms) + { + Q_ASSERT_X(this->hasStarted(), Q_FUNC_INFO, "Worker not started yet"); + m_updateTimer.startTimer(ms); + setEnabled(true); + } + void CBackgroundValidation::beforeQuit() noexcept { m_wasStopped = true; // stop in utility functions - this->stopUpdateTimer(); + m_updateTimer.stopTimer(); } void CBackgroundValidation::doWork() @@ -162,7 +170,7 @@ namespace swift::misc::simulation m_timerBasedRuns++; // stop timer after some runs - if (m_timerBasedRuns > 3) { m_updateTimer.stop(); } + if (m_timerBasedRuns > 3) { m_updateTimer.stopTimer(); } } emit this->validating(false); diff --git a/src/misc/simulation/backgroundvalidation.h b/src/misc/simulation/backgroundvalidation.h index 23926ba5d..45ed7e088 100644 --- a/src/misc/simulation/backgroundvalidation.h +++ b/src/misc/simulation/backgroundvalidation.h @@ -17,6 +17,7 @@ #include "misc/simulation/settings/modelmatchersettings.h" #include "misc/statusmessagelist.h" #include "misc/swiftmiscexport.h" +#include "misc/threadedtimer.h" #include "misc/worker.h" namespace swift::misc::simulation @@ -63,6 +64,10 @@ namespace swift::misc::simulation //! \threadsafe bool requestLastValidationResults(); + //! Start the updating timer + //! \threadsafe + void startUpdating(std::chrono::milliseconds ms); + signals: //! Validating void validating(bool running); @@ -96,6 +101,8 @@ namespace swift::misc::simulation // Set/caches as member as we are in own thread, central instance will not work data::CModelSetCaches m_modelSets { false, this }; + CThreadedTimer m_updateTimer; //!< update timer + //! Do the validation checks void doWork(); }; diff --git a/src/misc/threadedtimer.cpp b/src/misc/threadedtimer.cpp new file mode 100644 index 000000000..021c8b028 --- /dev/null +++ b/src/misc/threadedtimer.cpp @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: Copyright (C) 2025 swift Project Community / Contributors +// SPDX-License-Identifier: GPL-3.0-or-later OR LicenseRef-swift-pilot-client-1 + +#include "misc/threadedtimer.h" + +#include + +#include "threadutils.h" + +namespace swift::misc +{ + CThreadedTimer::CThreadedTimer(QObject *owner, const QString &name, bool singleShot) : QObject(owner) + { + m_updateTimer.setObjectName(name + ":timer"); + m_updateTimer.setSingleShot(singleShot); + connect(&m_updateTimer, &QTimer::timeout, this, &CThreadedTimer::timeout); + } + + void CThreadedTimer::startTimer(std::chrono::milliseconds ms) + { + if (!CThreadUtils::isInThisThread(this)) + { + // shift in correct thread + QPointer myself(this); + QTimer::singleShot(0, this, [=] { + if (!myself) { return; } + this->startTimer(ms); + }); + return; + } + + connect(thread(), &QThread::finished, &m_updateTimer, &QTimer::stop, Qt::UniqueConnection); + m_updateTimer.start(ms); + } + + void CThreadedTimer::stopTimer() + { + if (!CThreadUtils::isInThisThread(this)) + { + // shift in correct thread + QPointer myself(this); + QTimer::singleShot(0, this, [=] { + if (!myself) { return; } + this->stopTimer(); + }); + return; + } + + if (!m_updateTimer.isActive()) { return; } + m_updateTimer.stop(); + } +} // namespace swift::misc diff --git a/src/misc/threadedtimer.h b/src/misc/threadedtimer.h new file mode 100644 index 000000000..ded17e404 --- /dev/null +++ b/src/misc/threadedtimer.h @@ -0,0 +1,42 @@ +// SPDX-FileCopyrightText: Copyright (C) 2025 swift Project Community / Contributors +// SPDX-License-Identifier: GPL-3.0-or-later OR LicenseRef-swift-pilot-client-1 + +//! \file + +#ifndef SWIFT_MISC_THREADED_TIMER_H +#define SWIFT_MISC_THREADED_TIMER_H + +#include +#include + +#include "misc/swiftmiscexport.h" + +namespace swift::misc +{ + //! Thread-safe timer class + class SWIFT_MISC_EXPORT CThreadedTimer : public QObject + { + Q_OBJECT + public: + CThreadedTimer(QObject *owner, const QString& name, bool singleShot = false); + + //! Destructor + ~CThreadedTimer() override = default; + + //! Start updating (start timer) + //! \threadsafe + void startTimer(std::chrono::milliseconds ms); + + //! Safely stop update time + //! \threadsafe + void stopTimer(); + + signals: + void timeout(); + + private: + QTimer m_updateTimer { this }; //!< timer which can be used by implementing classes + }; +} // namespace swift::misc + +#endif // SWIFT_MISC_THREADED_TIMER_H diff --git a/src/misc/worker.cpp b/src/misc/worker.cpp index e90efd527..54a71ee54 100644 --- a/src/misc/worker.cpp +++ b/src/misc/worker.cpp @@ -160,7 +160,6 @@ namespace swift::misc { Q_ASSERT_X(!name.isEmpty(), Q_FUNC_INFO, "Empty name"); this->setObjectName(m_name); - m_updateTimer.setObjectName(m_name + ":timer"); } void CContinuousWorker::start(QThread::Priority priority) @@ -184,7 +183,6 @@ namespace swift::misc moveToThread(thread); connect(thread, &QThread::started, this, &CContinuousWorker::initialize); - connect(thread, &QThread::finished, &m_updateTimer, &QTimer::stop); connect(thread, &QThread::finished, this, &CContinuousWorker::cleanup); connect(thread, &QThread::finished, this, &CContinuousWorker::finish); thread->start(priority); @@ -232,50 +230,6 @@ namespace swift::misc Q_UNUSED(ok) } - void CContinuousWorker::startUpdating(int updateTimeSecs) - { - Q_ASSERT_X(this->hasStarted(), Q_FUNC_INFO, "Worker not yet started"); - if (!CThreadUtils::isInThisThread(this)) - { - // shift in correct thread - QPointer myself(this); - QTimer::singleShot(0, this, [=] { - if (!myself) { return; } - this->doIfNotFinished([=] { startUpdating(updateTimeSecs); }); - }); - return; - } - - // here in correct timer thread - if (updateTimeSecs < 0) - { - this->setEnabled(false); - m_updateTimer.stop(); - } - else - { - this->setEnabled(true); - m_updateTimer.start(1000 * updateTimeSecs); - } - } - - void CContinuousWorker::stopUpdateTimer() - { - if (!m_updateTimer.isActive()) { return; } - - // avoid "Timers cannot be stopped from another thread" - if (CThreadUtils::isInThisThread(&m_updateTimer)) { m_updateTimer.stop(); } - else - { - QPointer myself(this); - QTimer::singleShot(0, &m_updateTimer, [=] { - // stop timer in timer thread - if (!myself) { return; } - m_updateTimer.stop(); - }); - } - } - void CContinuousWorker::finish() { this->setFinished(); diff --git a/src/misc/worker.h b/src/misc/worker.h index ef6f07325..fe8df0d60 100644 --- a/src/misc/worker.h +++ b/src/misc/worker.h @@ -303,10 +303,6 @@ namespace swift::misc //! \threadsafe void setEnabled(bool enabled) { m_enabled = enabled; } - //! Start updating (start/stop timer) - //! \threadsafe - void startUpdating(int updateTimeSecs); - //! Name of the worker const QString &getName() { return m_name; } @@ -327,11 +323,6 @@ namespace swift::misc //! Wait time for quitAndWait, 0 means not waiting virtual unsigned long waitTimeoutMs() const { return 15 * 1000; } - //! Safely stop update time - void stopUpdateTimer(); - - QTimer m_updateTimer { this }; //!< timer which can be used by implementing classes - private: //! Called after cleanup(). void finish();