diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -175,6 +175,11 @@ #endif // HAVE_DECL_DAEMON } + // Lock data directory after daemonization + if (!AppInitLockDataDirectory()) { + // If locking the data directory failed, exit immediately + exit(EXIT_FAILURE); + } fRet = AppInitMain(config, httpRPCRequestProcessor, threadGroup, scheduler); } catch (const std::exception &e) { diff --git a/src/init.h b/src/init.h --- a/src/init.h +++ b/src/init.h @@ -1,5 +1,6 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto // Copyright (c) 2009-2016 The Bitcoin Core developers +// Copyright (c) 2018 The Bitcoin developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -28,14 +29,17 @@ //! Parameter interaction: change current parameters depending on various rules void InitParameterInteraction(); -/** Initialize bitcoin core: Basic context setup. - * @note This can be done before daemonization. - * @pre Parameters should be parsed and config file should be read. +/** + * Initialize bitcoin: Basic context setup. + * @note This can be done before daemonization. + * Do not call Shutdown() if this function fails. + * @pre Parameters should be parsed and config file should be read. */ bool AppInitBasicSetup(); /** * Initialization: parameter interaction. * @note This can be done before daemonization. + * Do not call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, * AppInitBasicSetup should have been called. */ @@ -43,16 +47,25 @@ /** * Initialization sanity checks: ecc init, sanity checks, dir lock. * @note This can be done before daemonization. + * Do not call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, * AppInitParameterInteraction should have been called. */ bool AppInitSanityChecks(); /** - * Bitcoin core main initialization. + * Lock bitcoin data directory. * @note This should only be done after daemonization. + * Do not call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, * AppInitSanityChecks should have been called. */ +bool AppInitLockDataDirectory(); +/** + * Bitcoin main initialization. + * @note This should only be done after daemonization. + * @pre Parameters should be parsed and config file should be read, + * AppInitLockDataDirectory should have been called. + */ bool AppInitMain(Config &config, HTTPRPCRequestProcessor &httpRPCRequestProcessor, boost::thread_group &threadGroup, CScheduler &scheduler); diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -1729,15 +1729,12 @@ } // Probe the data directory lock to give an early error message, if possible + // We cannot hold the data directory lock here, as the forking for daemon() + // hasn't yet happened, and a fork will cause weird behavior to it. return LockDataDirectory(true); } -bool AppInitMain(Config &config, - HTTPRPCRequestProcessor &httpRPCRequestProcessor, - boost::thread_group &threadGroup, CScheduler &scheduler) { - const CChainParams &chainparams = config.GetChainParams(); - // Step 4a: application initialization - +bool AppInitLockDataDirectory() { // After daemonization get the data directory lock again and hold on to it // until exit. This creates a slight window for a race condition to happen, // however this condition is harmless: it will at most make us exit without @@ -1746,6 +1743,14 @@ // Detailed error printed inside LockDataDirectory return false; } + return true; +} + +bool AppInitMain(Config &config, + HTTPRPCRequestProcessor &httpRPCRequestProcessor, + boost::thread_group &threadGroup, CScheduler &scheduler) { + // Step 4a: application initialization + const CChainParams &chainparams = config.GetChainParams(); #ifndef WIN32 CreatePidFile(GetPidFile(), getpid()); diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -169,10 +169,15 @@ public: explicit BitcoinABC(); + /** + * Basic initialization, before starting initialization/shutdown thread. + * Return true on success. + */ + static bool baseInitialize(Config &config, RPCServer &rpcServer); + public Q_SLOTS: void initialize(Config *config, - HTTPRPCRequestProcessor *httpRPCRequestProcessor, - RPCServer *rpcServer); + HTTPRPCRequestProcessor *httpRPCRequestProcessor); void shutdown(); Q_SIGNALS: @@ -262,28 +267,27 @@ Q_EMIT runawayException(QString::fromStdString(GetWarnings("gui"))); } +bool BitcoinABC::baseInitialize(Config &config, RPCServer &rpcServer) { + if (!AppInitBasicSetup()) { + return false; + } + if (!AppInitParameterInteraction(config, rpcServer)) { + return false; + } + if (!AppInitSanityChecks()) { + return false; + } + if (!AppInitLockDataDirectory()) { + return false; + } + return true; +} + void BitcoinABC::initialize(Config *cfg, - HTTPRPCRequestProcessor *httpRPCRequestProcessor, - RPCServer *rpcSrv) { + HTTPRPCRequestProcessor *httpRPCRequestProcessor) { Config &config(*cfg); - RPCServer &rpcServer = *rpcSrv; try { - qDebug() << __func__ << ": Running AppInit2 in thread"; - if (!AppInitBasicSetup()) { - Q_EMIT initializeResult(false); - return; - } - - if (!AppInitParameterInteraction(config, rpcServer)) { - Q_EMIT initializeResult(false); - return; - } - - if (!AppInitSanityChecks()) { - Q_EMIT initializeResult(false); - return; - } - + qDebug() << __func__ << ": Running initialization in thread"; bool rv = AppInitMain(config, *httpRPCRequestProcessor, threadGroup, scheduler); Q_EMIT initializeResult(rv); @@ -407,10 +411,9 @@ // temporary (eg it lives somewhere aside from the stack) or this will // crash because initialize() gets executed in another thread at some // unspecified time (after) requestedInitialize() is emitted! - connect(this, SIGNAL(requestedInitialize( - Config *, HTTPRPCRequestProcessor *, RPCServer *)), - executor, - SLOT(initialize(Config *, HTTPRPCRequestProcessor *, RPCServer *))); + connect(this, + SIGNAL(requestedInitialize(Config *, HTTPRPCRequestProcessor *)), + executor, SLOT(initialize(Config *, HTTPRPCRequestProcessor *))); connect(this, SIGNAL(requestedShutdown()), executor, SLOT(shutdown())); /* make sure executor object is deleted in its own thread */ @@ -765,18 +768,29 @@ RPCServer rpcServer; HTTPRPCRequestProcessor httpRPCRequestProcessor(config, rpcServer); + int rv = EXIT_SUCCESS; try { app.createWindow(&config, networkStyle.data()); - app.requestInitialize(config, httpRPCRequestProcessor, rpcServer); + // Perform base initialization before spinning up + // initialization/shutdown thread. This is acceptable because this + // function only contains steps that are quick to execute, so the GUI + // thread won't be held up. + if (BitcoinABC::baseInitialize(config, rpcServer)) { + app.requestInitialize(config, httpRPCRequestProcessor, rpcServer); #if defined(Q_OS_WIN) - WinShutdownMonitor::registerShutdownBlockReason( - QObject::tr("%1 didn't yet exit safely...") - .arg(QObject::tr(PACKAGE_NAME)), - (HWND)app.getMainWinId()); + WinShutdownMonitor::registerShutdownBlockReason( + QObject::tr("%1 didn't yet exit safely...") + .arg(QObject::tr(PACKAGE_NAME)), + (HWND)app.getMainWinId()); #endif - app.exec(); - app.requestShutdown(config); - app.exec(); + app.exec(); + app.requestShutdown(config); + app.exec(); + rv = app.getReturnValue(); + } else { + // A dialog with detailed error will have been shown by InitError() + rv = EXIT_FAILURE; + } } catch (const std::exception &e) { PrintExceptionContinue(&e, "Runaway exception"); app.handleRunawayException(QString::fromStdString(GetWarnings("gui"))); @@ -784,6 +798,6 @@ PrintExceptionContinue(nullptr, "Runaway exception"); app.handleRunawayException(QString::fromStdString(GetWarnings("gui"))); } - return app.getReturnValue(); + return rv; } #endif // BITCOIN_QT_TEST