From 518542f31fbeef558fa190a40db7aaeb33496c9d Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Tue, 13 Nov 2018 20:21:19 -0800 Subject: [PATCH] [Main] Unify shutdown proceedure in init rather than per-app This moves CScheduler and threadGroup to a static declaration in init .cpp so as to avoid a potential shutdown deadlock where both are freed before the network message handler thread has been completely released. --- src/init.cpp | 16 +++++++++++----- src/init.h | 4 ++-- src/pivxd.cpp | 20 +++++--------------- src/qt/pivx.cpp | 12 +++--------- 4 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 4f724803f918..77ebc1d6aed1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -171,14 +171,15 @@ static CCoinsViewDB* pcoinsdbview = NULL; static CCoinsViewErrorCatcher* pcoinscatcher = NULL; static boost::scoped_ptr globalVerifyHandle; -void Interrupt(boost::thread_group& threadGroup) +static boost::thread_group threadGroup; +static CScheduler scheduler; +void Interrupt() { InterruptHTTPServer(); InterruptHTTPRPC(); InterruptRPC(); InterruptREST(); InterruptTorControl(); - threadGroup.interrupt_all(); } /** Preparing steps before shutting down or restarting the wallet */ @@ -213,6 +214,11 @@ void PrepareShutdown() DumpMasternodePayments(); UnregisterNodeSignals(GetNodeSignals()); + // After everything has been shut down, but before things get flushed, stop the + // CScheduler/checkqueue threadGroup + threadGroup.interrupt_all(); + threadGroup.join_all(); + if (fFeeEstimatesInitialized) { boost::filesystem::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME; CAutoFile est_fileout(fopen(est_path.string().c_str(), "wb"), SER_DISK, CLIENT_VERSION); @@ -703,7 +709,7 @@ bool InitSanityCheck(void) return true; } -bool AppInitServers(boost::thread_group& threadGroup) +bool AppInitServers() { RPCServer::OnStopped(&OnRPCStopped); RPCServer::OnPreCommand(&OnRPCPreCommand); @@ -723,7 +729,7 @@ bool AppInitServers(boost::thread_group& threadGroup) /** Initialize pivx. * @pre Parameters should be parsed and config file should be read. */ -bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) +bool AppInit2() { // ********************************************************* Step 1: setup #ifdef _MSC_VER @@ -1053,7 +1059,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) */ if (fServer) { uiInterface.InitMessage.connect(SetRPCWarmupStatus); - if (!AppInitServers(threadGroup)) + if (!AppInitServers()) return InitError(_("Unable to start HTTP server. See debug log for details.")); } diff --git a/src/init.h b/src/init.h index 3270976d8e07..d6aa7d474ed4 100644 --- a/src/init.h +++ b/src/init.h @@ -24,10 +24,10 @@ extern CzPIVWallet* zwalletMain; void StartShutdown(); bool ShutdownRequested(); /** Interrupt threads */ -void Interrupt(boost::thread_group& threadGroup); +void Interrupt(); void Shutdown(); void PrepareShutdown(); -bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler); +bool AppInit2(); /** The help message mode determines what help message to show */ enum HelpMessageMode { diff --git a/src/pivxd.cpp b/src/pivxd.cpp index d921f3bd3cb6..b28428993ac6 100644 --- a/src/pivxd.cpp +++ b/src/pivxd.cpp @@ -10,7 +10,6 @@ #include "main.h" #include "masternodeconfig.h" #include "noui.h" -#include "scheduler.h" #include "rpc/server.h" #include "ui_interface.h" #include "util.h" @@ -41,7 +40,7 @@ static bool fDaemon; -void WaitForShutdown(boost::thread_group* threadGroup) +void WaitForShutdown() { bool fShutdown = ShutdownRequested(); // Tell the main threads to shutdown. @@ -49,10 +48,7 @@ void WaitForShutdown(boost::thread_group* threadGroup) MilliSleep(200); fShutdown = ShutdownRequested(); } - if (threadGroup) { - Interrupt(*threadGroup); - threadGroup->join_all(); - } + Interrupt(); } ////////////////////////////////////////////////////////////////////////////// @@ -61,9 +57,6 @@ void WaitForShutdown(boost::thread_group* threadGroup) // bool AppInit(int argc, char* argv[]) { - boost::thread_group threadGroup; - CScheduler scheduler; - bool fRet = false; // @@ -147,7 +140,7 @@ bool AppInit(int argc, char* argv[]) #endif SoftSetBoolArg("-server", true); - fRet = AppInit2(threadGroup, scheduler); + fRet = AppInit2(); } catch (std::exception& e) { PrintExceptionContinue(&e, "AppInit()"); } catch (...) { @@ -155,12 +148,9 @@ bool AppInit(int argc, char* argv[]) } if (!fRet) { - Interrupt(threadGroup); - // threadGroup.join_all(); was left out intentionally here, because we didn't re-test all of - // the startup-failure cases to make sure they don't result in a hang due to some - // thread-blocking-waiting-for-another-thread-during-startup case + Interrupt(); } else { - WaitForShutdown(&threadGroup); + WaitForShutdown(); } Shutdown(); diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index 85d03ce7602d..369ca11c6f61 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -30,7 +30,6 @@ #include "init.h" #include "main.h" #include "rpc/server.h" -#include "scheduler.h" #include "ui_interface.h" #include "util.h" @@ -166,9 +165,6 @@ public slots: void runawayException(const QString& message); private: - boost::thread_group threadGroup; - CScheduler scheduler; - /// Flag indicating a restart bool execute_restart; @@ -252,7 +248,7 @@ void BitcoinCore::initialize() try { qDebug() << __func__ << ": Running AppInit2 in thread"; - int rv = AppInit2(threadGroup, scheduler); + int rv = AppInit2(); emit initializeResult(rv); } catch (std::exception& e) { handleRunawayException(&e); @@ -267,8 +263,7 @@ void BitcoinCore::restart(QStringList args) execute_restart = false; try { qDebug() << __func__ << ": Running Restart in thread"; - Interrupt(threadGroup); - threadGroup.join_all(); + Interrupt(); PrepareShutdown(); qDebug() << __func__ << ": Shutdown finished"; emit shutdownResult(1); @@ -288,8 +283,7 @@ void BitcoinCore::shutdown() { try { qDebug() << __func__ << ": Running Shutdown in thread"; - Interrupt(threadGroup); - threadGroup.join_all(); + Interrupt(); Shutdown(); qDebug() << __func__ << ": Shutdown finished"; emit shutdownResult(1);