From 6fb74ead608a7aa36eb82a8ab655e683564db752 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 17 Jul 2017 17:12:00 +0200 Subject: [PATCH] Merge #10832: init: Factor out AppInitLockDataDirectory and fix startup core dump issue dba485d init: Factor out AppInitLockDataDirectory (Wladimir J. van der Laan) Pull request description: Alternative to #10818, alternative solution to #10815. After this change: All the AppInit steps before and inclusive AppInitLockDataDirectory must not have Shutdown() called in case of failure. Only when AppInitMain fails, Shutdown should be called. Changes the GUI and bitcoind code to consistently do this. Tree-SHA512: 393e1a0ae05eb8e791025069e3ac4f6f3cdeb459ec63feda85d01cf6696ab3fed7632b6a0ac3641b8c7015af51d46756b5bba77f5e5f0c446f0c2dea58bbc92e --- src/dashd.cpp | 7 +++++- src/init.cpp | 12 +++++++--- src/init.h | 16 +++++++++---- src/qt/dash.cpp | 62 ++++++++++++++++++++++++++++++++----------------- 4 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/dashd.cpp b/src/dashd.cpp index 06e4a0ef9c..dbe9e71395 100644 --- a/src/dashd.cpp +++ b/src/dashd.cpp @@ -172,7 +172,12 @@ bool AppInit(int argc, char* argv[]) return false; #endif // HAVE_DECL_DAEMON } - + // Lock data directory after daemonization + if (!AppInitLockDataDirectory()) + { + // If locking the data directory failed, exit immediately + exit(EXIT_FAILURE); + } fRet = AppInitMain(threadGroup, scheduler); } catch (...) { PrintExceptionContinue(std::current_exception(), "AppInit()"); diff --git a/src/init.cpp b/src/init.cpp index fa8aebda59..cd4dd99c4c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1453,13 +1453,13 @@ bool AppInitSanityChecks() return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), _(PACKAGE_NAME))); // 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(boost::thread_group& threadGroup, CScheduler& scheduler) +bool AppInitLockDataDirectory() { - const CChainParams& chainparams = Params(); - // ********************************************************* Step 4a: application initialization // 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 printing a message to console. @@ -1467,7 +1467,13 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // Detailed error printed inside LockDataDirectory return false; } + return true; +} +bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) +{ + const CChainParams& chainparams = Params(); + // ********************************************************* Step 4a: application initialization #ifndef WIN32 CreatePidFile(GetPidFile(), getpid()); #endif diff --git a/src/init.h b/src/init.h index 0706895f33..36315aefcc 100644 --- a/src/init.h +++ b/src/init.h @@ -28,27 +28,33 @@ void InitLogging(); void InitParameterInteraction(); /** Initialize bitcoin core: Basic context setup. - * @note This can be done before daemonization. + * @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. + * @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. */ bool AppInitParameterInteraction(); /** * Initialization sanity checks: ecc init, sanity checks, dir lock. - * @note This can be done before daemonization. + * @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. - * @note This should only be done after daemonization. + * Lock bitcoin core 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 core main initialization. + * @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); void PrepareShutdown(); diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index b538367fb7..48b20fef0e 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -181,6 +181,10 @@ class BitcoinCore: public QObject Q_OBJECT public: explicit BitcoinCore(); + /** Basic initialization, before starting initialization/shutdown thread. + * Return true on success. + */ + static bool baseInitialize(); public Q_SLOTS: void initialize(); @@ -275,26 +279,32 @@ void BitcoinCore::handleRunawayException(const std::exception_ptr e) Q_EMIT runawayException(QString::fromStdString(GetWarnings("gui"))); } +bool BitcoinCore::baseInitialize() +{ + if (!AppInitBasicSetup()) + { + return false; + } + if (!AppInitParameterInteraction()) + { + return false; + } + if (!AppInitSanityChecks()) + { + return false; + } + if (!AppInitLockDataDirectory()) + { + return false; + } + return true; +} + void BitcoinCore::initialize() { try { qDebug() << __func__ << ": Running initialization in thread"; - if (!AppInitBasicSetup()) - { - Q_EMIT initializeResult(false); - return; - } - if (!AppInitParameterInteraction()) - { - Q_EMIT initializeResult(false); - return; - } - if (!AppInitSanityChecks()) - { - Q_EMIT initializeResult(false); - return; - } bool rv = AppInitMain(threadGroup, scheduler); Q_EMIT initializeResult(rv); } catch (...) { @@ -725,20 +735,30 @@ int main(int argc, char *argv[]) if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false)) app.createSplashScreen(networkStyle.data()); + int rv = EXIT_SUCCESS; try { app.createWindow(networkStyle.data()); - app.requestInitialize(); + // 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 (BitcoinCore::baseInitialize()) { + app.requestInitialize(); #if defined(Q_OS_WIN) && QT_VERSION >= 0x050000 - 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(); - app.exec(); + app.exec(); + app.requestShutdown(); + app.exec(); + rv = app.getReturnValue(); + } else { + // A dialog with detailed error will have been shown by InitError() + rv = EXIT_FAILURE; + } } catch (...) { PrintExceptionContinue(std::current_exception(), "Runaway exception"); app.handleRunawayException(QString::fromStdString(GetWarnings("gui"))); } - return app.getReturnValue(); + return rv; } #endif // BITCOIN_QT_TEST