From d6fd53287fe12e7e4eeefdeb5572818bb5203e14 Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Mon, 19 Mar 2018 21:04:17 +0100 Subject: [PATCH] Improved graceful shutdown, added "m_shutdownInProgress" * no assert when wait is called in same thread, just ignore wait * quitAndWait readers, also works if not already noved in new thread (see above) --- src/blackcore/application.cpp | 17 +++++++++----- src/blackcore/application.h | 3 ++- src/blackcore/db/networkwatchdog.cpp | 1 + src/blackcore/webdataservices.cpp | 29 +++++++++++++++--------- src/blackcore/webdataservices.h | 3 ++- src/blackgui/guiapplication.cpp | 8 +++++-- src/blackmisc/worker.cpp | 33 ++++++++++++++++++++-------- src/blackmisc/worker.h | 6 +++++ 8 files changed, 71 insertions(+), 29 deletions(-) diff --git a/src/blackcore/application.cpp b/src/blackcore/application.cpp index 0831a8281..4bdea8a3d 100644 --- a/src/blackcore/application.cpp +++ b/src/blackcore/application.cpp @@ -105,7 +105,7 @@ namespace BlackCore CApplication::CApplication(const QString &applicationName, CApplicationInfo::Application application, bool init) : m_accessManager(new QNetworkAccessManager(this)), m_applicationInfo(application), - m_cookieManager( {}, this), m_applicationName(applicationName), m_coreFacadeConfig(CCoreFacadeConfig::allEmpty()) + m_cookieManager({}, this), m_applicationName(applicationName), m_coreFacadeConfig(CCoreFacadeConfig::allEmpty()) { Q_ASSERT_X(!sApp, Q_FUNC_INFO, "already initialized"); Q_ASSERT_X(QCoreApplication::instance(), Q_FUNC_INFO, "no application object"); @@ -125,7 +125,13 @@ namespace BlackCore { if (!sApp) { + // notify when app goes down + connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &CApplication::gracefulShutdown); + + // metadata if (withMetadata) { CApplication::registerMetadata(); } + + // unit test if (this->getApplicationInfo().application() == CApplicationInfo::UnitTest) { const QString tempPath(this->getTemporaryDirectory()); @@ -174,9 +180,6 @@ namespace BlackCore // startup done connect(this, &CApplication::startUpCompleted, this, &CApplication::onStartUpCompleted, Qt::QueuedConnection); connect(this, &CApplication::coreFacadeStarted, this, &CApplication::onCoreFacadeStarted, Qt::QueuedConnection); - - // notify when app goes down - connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this, &CApplication::gracefulShutdown); } } @@ -751,7 +754,7 @@ namespace BlackCore void CApplication::exit(int retcode) { - if (instance()) { instance()->gracefulShutdown(); } + if (sApp) { instance()->gracefulShutdown(); } // when the event loop is not running, this does nothing QCoreApplication::exit(retcode); @@ -956,8 +959,10 @@ namespace BlackCore void CApplication::gracefulShutdown() { if (m_shutdown) { return; } + if (m_shutdownInProgress) { return; } + m_shutdownInProgress = true; - // before marked as shutdown + // before marked as shutdown, otherwise URL if (m_networkWatchDog) { m_networkWatchDog->gracefulShutdown(); diff --git a/src/blackcore/application.h b/src/blackcore/application.h index 01cf38c83..83710e2df 100644 --- a/src/blackcore/application.h +++ b/src/blackcore/application.h @@ -564,6 +564,8 @@ namespace BlackCore bool m_started = false; //!< started with success? bool m_singleApplication = true; //!< only one instance of that application bool m_alreadyRunning = false; //!< Application already running + std::atomic_bool m_shutdown { false }; //!< is being shutdown? + std::atomic_bool m_shutdownInProgress { false }; //!< shutdown in progress? private: //! Problem with network access manager @@ -611,7 +613,6 @@ namespace BlackCore CCoreFacadeConfig m_coreFacadeConfig; //!< Core facade config if any CWebReaderFlags::WebReader m_webReadersUsed; //!< Readers to be used Db::CDatabaseReaderConfigList m_dbReaderConfig; //!< Load or used caching? - std::atomic m_shutdown { false }; //!< is being shutdown? bool m_useContexts = false; //!< use contexts bool m_useWebData = false; //!< use web data bool m_signalStartup = true; //!< signal startup automatically diff --git a/src/blackcore/db/networkwatchdog.cpp b/src/blackcore/db/networkwatchdog.cpp index 2bd469ff2..588da4f4a 100644 --- a/src/blackcore/db/networkwatchdog.cpp +++ b/src/blackcore/db/networkwatchdog.cpp @@ -207,6 +207,7 @@ namespace BlackCore void CNetworkWatchdog::gracefulShutdown() { this->pingDbClientService(PingCompleteShutdown); + this->quit(); } void CNetworkWatchdog::pingDbClientService(CNetworkWatchdog::PingType type, bool force) diff --git a/src/blackcore/webdataservices.cpp b/src/blackcore/webdataservices.cpp index 6e65787a5..8b158768a 100644 --- a/src/blackcore/webdataservices.cpp +++ b/src/blackcore/webdataservices.cpp @@ -892,19 +892,20 @@ namespace BlackCore void CWebDataServices::gracefulShutdown() { + if (m_shuttingDown) { return; } m_shuttingDown = true; this->disconnect(); // all signals - if (m_vatsimMetarReader) { m_vatsimMetarReader->setEnabled(false); } - if (m_vatsimBookingReader) { m_vatsimBookingReader->setEnabled(false); } - if (m_vatsimDataFileReader) { m_vatsimDataFileReader->setEnabled(false); } - if (m_vatsimStatusReader) { m_vatsimStatusReader->setEnabled(false); } - if (m_modelDataReader) { m_modelDataReader->setEnabled(false); } - if (m_airportDataReader) { m_airportDataReader->setEnabled(false); } - if (m_icaoDataReader) { m_icaoDataReader->setEnabled(false); } - if (m_dbInfoDataReader) { m_dbInfoDataReader->setEnabled(false); } + if (m_vatsimMetarReader) { m_vatsimMetarReader->quitAndWait(); m_vatsimMetarReader = nullptr; } + if (m_vatsimBookingReader) { m_vatsimBookingReader->quitAndWait(); m_vatsimBookingReader = nullptr; } + if (m_vatsimDataFileReader) { m_vatsimDataFileReader->quitAndWait(); m_vatsimDataFileReader = nullptr; } + if (m_vatsimStatusReader) { m_vatsimStatusReader->quitAndWait(); m_vatsimStatusReader = nullptr; } + if (m_modelDataReader) { m_modelDataReader->quitAndWait(); m_modelDataReader = nullptr; } + if (m_airportDataReader) { m_airportDataReader->quitAndWait(); m_airportDataReader = nullptr; } + if (m_icaoDataReader) { m_icaoDataReader->quitAndWait(); m_icaoDataReader = nullptr; } + if (m_dbInfoDataReader) { m_dbInfoDataReader->quitAndWait(); m_dbInfoDataReader = nullptr; } // DB writer is no threaded reader, it has a special role - if (m_databaseWriter) { m_databaseWriter->gracefulShutdown(); } + if (m_databaseWriter) { m_databaseWriter->gracefulShutdown(); m_databaseWriter = nullptr; } } CEntityFlags::Entity CWebDataServices::allDbEntitiesForUsedReaders() const @@ -1287,6 +1288,7 @@ namespace BlackCore void CWebDataServices::readDeferredInBackground(CEntityFlags::Entity entities, int delayMs) { + if (m_shuttingDown) { return; } if (entities == CEntityFlags::NoEntity) { return; } QTimer::singleShot(delayMs, [ = ]() { @@ -1296,6 +1298,8 @@ namespace BlackCore void CWebDataServices::readInBackground(CEntityFlags::Entity entities) { + if (m_shuttingDown) { return; } + m_initialRead = true; // read started if (CEntityFlags::anySwiftDbEntity(entities)) { @@ -1324,6 +1328,8 @@ namespace BlackCore bool CWebDataServices::waitForDbInfoObjectsThenRead(CEntityFlags::Entity entities) { + if (m_shuttingDown) { return false; } + Q_ASSERT_X(m_dbInfoDataReader, Q_FUNC_INFO, "need reader"); if (m_dbInfoDataReader->areAllInfoObjectsRead()) { return true; } if (!m_dbInfoObjectTimeout.isValid()) { m_dbInfoObjectTimeout = QDateTime::currentDateTimeUtc().addMSecs(10 * 1000); } @@ -1333,6 +1339,8 @@ namespace BlackCore bool CWebDataServices::waitForSharedInfoObjectsThenRead(CEntityFlags::Entity entities) { + if (m_shuttingDown) { return false; } + Q_ASSERT_X(m_sharedInfoDataReader, Q_FUNC_INFO, "need reader"); if (m_sharedInfoDataReader->areAllInfoObjectsRead()) { return true; } if (!m_sharedInfoObjectsTimeout.isValid()) { m_sharedInfoObjectsTimeout = QDateTime::currentDateTimeUtc().addMSecs(10 * 1000); } @@ -1342,9 +1350,10 @@ namespace BlackCore bool CWebDataServices::waitForInfoObjectsThenRead(CEntityFlags::Entity entities, const QString &info, CInfoDataReader *infoReader, QDateTime &timeOut) { - Q_ASSERT_X(infoReader, Q_FUNC_INFO, "Need info data reader"); + if (m_shuttingDown) { return false; } // this will called for each entity readers, i.e. model reader, ICAO reader ... + Q_ASSERT_X(infoReader, Q_FUNC_INFO, "Need info data reader"); const int waitForInfoObjectsMs = 1000; // ms if (infoReader->areAllInfoObjectsRead()) diff --git a/src/blackcore/webdataservices.h b/src/blackcore/webdataservices.h index 28229c76b..a57df52d6 100644 --- a/src/blackcore/webdataservices.h +++ b/src/blackcore/webdataservices.h @@ -42,6 +42,7 @@ #include #include #include +#include namespace BlackMisc { @@ -580,7 +581,7 @@ namespace BlackCore BlackCore::Db::CDatabaseReaderConfigList m_dbReaderConfig; //!< how to read DB data bool m_initialRead = false; //!< initial read started bool m_signalledHeaders = false; //!< headers loading has been signalled - bool m_shuttingDown = false; //!< shutting down? + std::atomic_bool m_shuttingDown { false }; //!< shutting down? QDateTime m_dbInfoObjectTimeout; //!< started reading DB info objects QDateTime m_sharedInfoObjectsTimeout; //!< started reading shared info objects QSet m_signalledEntities; //!< remember signalled entites diff --git a/src/blackgui/guiapplication.cpp b/src/blackgui/guiapplication.cpp index 04d82daab..d1d9d31f6 100644 --- a/src/blackgui/guiapplication.cpp +++ b/src/blackgui/guiapplication.cpp @@ -34,10 +34,10 @@ #include #include #include -#include #include #include #include +#include #include #include #include @@ -86,6 +86,9 @@ namespace BlackGui this->addWindowModeOption(); this->addWindowResetSizeOption(); + // notify when app goes down + connect(qGuiApp, &QGuiApplication::lastWindowClosed, this, &CGuiApplication::gracefulShutdown); + if (!sGui) { CGuiApplication::registerMetadata(); @@ -879,11 +882,12 @@ namespace BlackGui void CGuiApplication::gracefulShutdown() { + if (m_shutdownInProgress) { return; } + CApplication::gracefulShutdown(); if (m_saveMainWidgetState) { this->saveWindowGeometryAndState(); } - CApplication::gracefulShutdown(); } void CGuiApplication::settingsChanged() diff --git a/src/blackmisc/worker.cpp b/src/blackmisc/worker.cpp index c35059d2b..62dee78db 100644 --- a/src/blackmisc/worker.cpp +++ b/src/blackmisc/worker.cpp @@ -36,7 +36,7 @@ namespace BlackMisc auto handle = m_handle.load(); if (handle) { - auto status = WaitForSingleObject(handle, 0); + const auto status = WaitForSingleObject(handle, 0); if (isRunning()) { switch (status) @@ -50,7 +50,12 @@ namespace BlackMisc } #endif quit(); + + const qint64 beforeWait = QDateTime::currentMSecsSinceEpoch(); wait(30 * 1000); //! \todo KB 2017-10 temp workaround: in T145 this will be fixed, sometimes (very rarely) hanging here during shutdown + const qint64 delta = QDateTime::currentMSecsSinceEpoch() - beforeWait; + BLACK_VERIFY_X(delta < 30 * 1000, Q_FUNC_INFO, "Wait timeout"); + Q_UNUSED(delta); } CWorker *CWorker::fromTaskImpl(QObject *owner, const QString &name, int typeId, std::function task) @@ -67,7 +72,7 @@ namespace BlackMisc worker->setObjectName(name); worker->moveToThread(thread); - bool s = QMetaObject::invokeMethod(worker, "ps_runTask"); + const bool s = QMetaObject::invokeMethod(worker, "ps_runTask"); Q_ASSERT_X(s, Q_FUNC_INFO, "cannot invoke"); Q_UNUSED(s); thread->start(); @@ -153,21 +158,31 @@ namespace BlackMisc void CContinuousWorker::quit() noexcept { - Q_ASSERT_X(!CThreadUtils::isApplicationThreadObjectThread(this), Q_FUNC_INFO, "Try to stop main thread"); - setEnabled(false); + this->setEnabled(false); + + // already in different thread? otherwise return + if (CThreadUtils::isApplicationThreadObjectThread(this)) { return; } + // remark: cannot stop timer here, as I am normally not in the correct thread thread()->quit(); } void CContinuousWorker::quitAndWait() noexcept { - Q_ASSERT_X(!CThreadUtils::isApplicationThreadObjectThread(this), Q_FUNC_INFO, "Try to stop main thread"); - Q_ASSERT_X(!CThreadUtils::isCurrentThreadObjectThread(this), Q_FUNC_INFO, "Called by own thread, will deadlock"); + this->quit(); - setEnabled(false); + // already in different thread? otherwise return + if (CThreadUtils::isApplicationThreadObjectThread(this)) { return; } + + // Called by own thread, will deadlock, return + if (CThreadUtils::isCurrentThreadObjectThread(this)) { return; } auto *ownThread = thread(); - quit(); - ownThread->wait(); + + const qint64 beforeWait = QDateTime::currentMSecsSinceEpoch(); + ownThread->wait(30 * 1000); //! \todo KB 2017-10 temp workaround: in T145 this will be fixed, sometimes (very rarely) hanging here during shutdown + const qint64 delta = QDateTime::currentMSecsSinceEpoch() - beforeWait; + BLACK_VERIFY_X(delta < 30 * 1000, Q_FUNC_INFO, "Wait timeout"); + Q_UNUSED(delta); } void CContinuousWorker::startUpdating(int updateTimeSecs) diff --git a/src/blackmisc/worker.h b/src/blackmisc/worker.h index 8bd04609c..2f72f7a88 100644 --- a/src/blackmisc/worker.h +++ b/src/blackmisc/worker.h @@ -315,6 +315,12 @@ namespace BlackMisc //! Called when the thread is finished. virtual void cleanup() {} + //! Owner of the worker + //! @{ + const QObject *owner() const { return m_owner; } + QObject *owner() { return m_owner; } + //! @} + QTimer m_updateTimer { this }; //!< timer which can be used by implementing classes private: