Merge #12266: Move scheduler/threadGroup into common-init instead of per-app

082a61c Move scheduler/threadGroup into common-init instead of per-app (Matt Corallo)

Pull request description:

  This resolves #12229 which pointed out a shutdown deadlock due to
  scheduler/checkqueue having been shut down while network message
  processing is still running.

Tree-SHA512: 0c0a76113996b164b0610d3b8c40b396f3e384d165bf098768e31fe3701b00763d0d810ef24702387e2e936fefb9fb900a6225f7417bb0175b585f365d542660
This commit is contained in:
Wladimir J. van der Laan 2018-01-30 12:34:11 +01:00
commit 3448907a68
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
4 changed files with 22 additions and 27 deletions

View file

@ -14,7 +14,6 @@
#include <rpc/server.h> #include <rpc/server.h>
#include <init.h> #include <init.h>
#include <noui.h> #include <noui.h>
#include <scheduler.h>
#include <util.h> #include <util.h>
#include <httpserver.h> #include <httpserver.h>
#include <httprpc.h> #include <httprpc.h>
@ -40,7 +39,7 @@
* Use the buttons <code>Namespaces</code>, <code>Classes</code> or <code>Files</code> at the top of the page to start navigating the code. * Use the buttons <code>Namespaces</code>, <code>Classes</code> or <code>Files</code> at the top of the page to start navigating the code.
*/ */
void WaitForShutdown(boost::thread_group* threadGroup) void WaitForShutdown()
{ {
bool fShutdown = ShutdownRequested(); bool fShutdown = ShutdownRequested();
// Tell the main threads to shutdown. // Tell the main threads to shutdown.
@ -49,11 +48,7 @@ void WaitForShutdown(boost::thread_group* threadGroup)
MilliSleep(200); MilliSleep(200);
fShutdown = ShutdownRequested(); fShutdown = ShutdownRequested();
} }
if (threadGroup) Interrupt();
{
Interrupt(*threadGroup);
threadGroup->join_all();
}
} }
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
@ -62,9 +57,6 @@ void WaitForShutdown(boost::thread_group* threadGroup)
// //
bool AppInit(int argc, char* argv[]) bool AppInit(int argc, char* argv[])
{ {
boost::thread_group threadGroup;
CScheduler scheduler;
bool fRet = false; bool fRet = false;
// //
@ -165,7 +157,7 @@ bool AppInit(int argc, char* argv[])
// If locking the data directory failed, exit immediately // If locking the data directory failed, exit immediately
return false; return false;
} }
fRet = AppInitMain(threadGroup, scheduler); fRet = AppInitMain();
} }
catch (const std::exception& e) { catch (const std::exception& e) {
PrintExceptionContinue(&e, "AppInit()"); PrintExceptionContinue(&e, "AppInit()");
@ -175,10 +167,9 @@ bool AppInit(int argc, char* argv[])
if (!fRet) if (!fRet)
{ {
Interrupt(threadGroup); Interrupt();
threadGroup.join_all();
} else { } else {
WaitForShutdown(&threadGroup); WaitForShutdown();
} }
Shutdown(); Shutdown();

View file

@ -155,7 +155,10 @@ public:
static std::unique_ptr<CCoinsViewErrorCatcher> pcoinscatcher; static std::unique_ptr<CCoinsViewErrorCatcher> pcoinscatcher;
static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle; static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle;
void Interrupt(boost::thread_group& threadGroup) static boost::thread_group threadGroup;
static CScheduler scheduler;
void Interrupt()
{ {
InterruptHTTPServer(); InterruptHTTPServer();
InterruptHTTPRPC(); InterruptHTTPRPC();
@ -164,7 +167,6 @@ void Interrupt(boost::thread_group& threadGroup)
InterruptTorControl(); InterruptTorControl();
if (g_connman) if (g_connman)
g_connman->Interrupt(); g_connman->Interrupt();
threadGroup.interrupt_all();
} }
void Shutdown() void Shutdown()
@ -199,6 +201,12 @@ void Shutdown()
g_connman.reset(); g_connman.reset();
StopTorControl(); 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)) { if (fDumpMempoolLater && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
DumpMempool(); DumpMempool();
} }
@ -705,7 +713,7 @@ bool InitSanityCheck(void)
return true; return true;
} }
bool AppInitServers(boost::thread_group& threadGroup) bool AppInitServers()
{ {
RPCServer::OnStarted(&OnRPCStarted); RPCServer::OnStarted(&OnRPCStarted);
RPCServer::OnStopped(&OnRPCStopped); RPCServer::OnStopped(&OnRPCStopped);
@ -1190,7 +1198,7 @@ bool AppInitLockDataDirectory()
return true; return true;
} }
bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) bool AppInitMain()
{ {
const CChainParams& chainparams = Params(); const CChainParams& chainparams = Params();
// ********************************************************* Step 4a: application initialization // ********************************************************* Step 4a: application initialization
@ -1257,7 +1265,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
if (gArgs.GetBoolArg("-server", false)) if (gArgs.GetBoolArg("-server", false))
{ {
uiInterface.InitMessage.connect(SetRPCWarmupStatus); uiInterface.InitMessage.connect(SetRPCWarmupStatus);
if (!AppInitServers(threadGroup)) if (!AppInitServers())
return InitError(_("Unable to start HTTP server. See debug log for details.")); return InitError(_("Unable to start HTTP server. See debug log for details."));
} }

View file

@ -19,7 +19,7 @@ class thread_group;
void StartShutdown(); void StartShutdown();
bool ShutdownRequested(); bool ShutdownRequested();
/** Interrupt threads */ /** Interrupt threads */
void Interrupt(boost::thread_group& threadGroup); void Interrupt();
void Shutdown(); void Shutdown();
//!Initialize the logging infrastructure //!Initialize the logging infrastructure
void InitLogging(); void InitLogging();
@ -54,7 +54,7 @@ bool AppInitLockDataDirectory();
* @note This should only be done after daemonization. Call Shutdown() if this function fails. * @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. * @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 */ /** The help message mode determines what help message to show */
enum HelpMessageMode { enum HelpMessageMode {

View file

@ -28,7 +28,6 @@
#include <init.h> #include <init.h>
#include <rpc/server.h> #include <rpc/server.h>
#include <scheduler.h>
#include <ui_interface.h> #include <ui_interface.h>
#include <util.h> #include <util.h>
#include <warnings.h> #include <warnings.h>
@ -193,8 +192,6 @@ Q_SIGNALS:
void runawayException(const QString &message); void runawayException(const QString &message);
private: private:
boost::thread_group threadGroup;
CScheduler scheduler;
/// Pass fatal exception message to UI thread /// Pass fatal exception message to UI thread
void handleRunawayException(const std::exception *e); void handleRunawayException(const std::exception *e);
@ -300,7 +297,7 @@ void BitcoinCore::initialize()
try try
{ {
qDebug() << __func__ << ": Running initialization in thread"; qDebug() << __func__ << ": Running initialization in thread";
bool rv = AppInitMain(threadGroup, scheduler); bool rv = AppInitMain();
Q_EMIT initializeResult(rv); Q_EMIT initializeResult(rv);
} catch (const std::exception& e) { } catch (const std::exception& e) {
handleRunawayException(&e); handleRunawayException(&e);
@ -314,8 +311,7 @@ void BitcoinCore::shutdown()
try try
{ {
qDebug() << __func__ << ": Running Shutdown in thread"; qDebug() << __func__ << ": Running Shutdown in thread";
Interrupt(threadGroup); Interrupt();
threadGroup.join_all();
Shutdown(); Shutdown();
qDebug() << __func__ << ": Shutdown finished"; qDebug() << __func__ << ": Shutdown finished";
Q_EMIT shutdownResult(); Q_EMIT shutdownResult();