From 082a61c69d7a539b5a48c4376a657f1c5aa92d81 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 24 Jan 2018 21:15:56 -0500 Subject: [PATCH] Move scheduler/threadGroup into common-init instead of per-app This resolves #12229 which pointed out a shutdown deadlock due to scheduler/checkqueue having been shut down while network message processing is still running. --- src/bitcoind.cpp | 19 +++++-------------- src/init.cpp | 18 +++++++++++++----- src/init.h | 4 ++-- src/qt/bitcoin.cpp | 8 ++------ 4 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index de19787a16936..d3eb60725f4af 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -40,7 +39,7 @@ * Use the buttons Namespaces, Classes or Files at the top of the page to start navigating the code. */ -void WaitForShutdown(boost::thread_group* threadGroup) +void WaitForShutdown() { bool fShutdown = ShutdownRequested(); // Tell the main threads to shutdown. @@ -49,11 +48,7 @@ void WaitForShutdown(boost::thread_group* threadGroup) MilliSleep(200); fShutdown = ShutdownRequested(); } - if (threadGroup) - { - Interrupt(*threadGroup); - threadGroup->join_all(); - } + Interrupt(); } ////////////////////////////////////////////////////////////////////////////// @@ -62,9 +57,6 @@ void WaitForShutdown(boost::thread_group* threadGroup) // bool AppInit(int argc, char* argv[]) { - boost::thread_group threadGroup; - CScheduler scheduler; - bool fRet = false; // @@ -165,7 +157,7 @@ bool AppInit(int argc, char* argv[]) // If locking the data directory failed, exit immediately return false; } - fRet = AppInitMain(threadGroup, scheduler); + fRet = AppInitMain(); } catch (const std::exception& e) { PrintExceptionContinue(&e, "AppInit()"); @@ -175,10 +167,9 @@ bool AppInit(int argc, char* argv[]) if (!fRet) { - Interrupt(threadGroup); - threadGroup.join_all(); + Interrupt(); } else { - WaitForShutdown(&threadGroup); + WaitForShutdown(); } Shutdown(); diff --git a/src/init.cpp b/src/init.cpp index d1d733af94e6a..b9c78b821174d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -155,7 +155,10 @@ class CCoinsViewErrorCatcher final : public CCoinsViewBacked static std::unique_ptr pcoinscatcher; static std::unique_ptr globalVerifyHandle; -void Interrupt(boost::thread_group& threadGroup) +static boost::thread_group threadGroup; +static CScheduler scheduler; + +void Interrupt() { InterruptHTTPServer(); InterruptHTTPRPC(); @@ -164,7 +167,6 @@ void Interrupt(boost::thread_group& threadGroup) InterruptTorControl(); if (g_connman) g_connman->Interrupt(); - threadGroup.interrupt_all(); } void Shutdown() @@ -199,6 +201,12 @@ void Shutdown() g_connman.reset(); StopTorControl(); + + // After everything has been shut down, but before things get flushed, stop the + // CScheduler/checkqueue threadGroup + threadGroup.interrupt_all(); + threadGroup.join_all(); + if (fDumpMempoolLater && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { DumpMempool(); } @@ -705,7 +713,7 @@ bool InitSanityCheck(void) return true; } -bool AppInitServers(boost::thread_group& threadGroup) +bool AppInitServers() { RPCServer::OnStarted(&OnRPCStarted); RPCServer::OnStopped(&OnRPCStopped); @@ -1184,7 +1192,7 @@ bool AppInitLockDataDirectory() return true; } -bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) +bool AppInitMain() { const CChainParams& chainparams = Params(); // ********************************************************* Step 4a: application initialization @@ -1251,7 +1259,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) if (gArgs.GetBoolArg("-server", false)) { 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 843024f02b202..33f97a55a570d 100644 --- a/src/init.h +++ b/src/init.h @@ -19,7 +19,7 @@ class thread_group; void StartShutdown(); bool ShutdownRequested(); /** Interrupt threads */ -void Interrupt(boost::thread_group& threadGroup); +void Interrupt(); void Shutdown(); //!Initialize the logging infrastructure void InitLogging(); @@ -54,7 +54,7 @@ bool AppInitLockDataDirectory(); * @note This should only be done after daemonization. Call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. */ -bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler); +bool AppInitMain(); /** The help message mode determines what help message to show */ enum HelpMessageMode { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 0666dcb9a3ead..b26d99a20a1a3 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -28,7 +28,6 @@ #include #include -#include #include #include #include @@ -193,8 +192,6 @@ public Q_SLOTS: void runawayException(const QString &message); private: - boost::thread_group threadGroup; - CScheduler scheduler; /// Pass fatal exception message to UI thread void handleRunawayException(const std::exception *e); @@ -300,7 +297,7 @@ void BitcoinCore::initialize() try { qDebug() << __func__ << ": Running initialization in thread"; - bool rv = AppInitMain(threadGroup, scheduler); + bool rv = AppInitMain(); Q_EMIT initializeResult(rv); } catch (const std::exception& e) { handleRunawayException(&e); @@ -314,8 +311,7 @@ void BitcoinCore::shutdown() try { qDebug() << __func__ << ": Running Shutdown in thread"; - Interrupt(threadGroup); - threadGroup.join_all(); + Interrupt(); Shutdown(); qDebug() << __func__ << ": Shutdown finished"; Q_EMIT shutdownResult();