From dcc9dffae9f52d3f2978e6a57f3b54fd06bdbace Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 19 Dec 2016 12:38:35 +0100 Subject: [PATCH] Merge #9236: Fix races for strMiscWarning and fLargeWork*Found, make QT runawayException use GetWarnings 749be01 Move GetWarnings() into its own file. (Gregory Maxwell) e3ba0ef Eliminate data races for strMiscWarning and fLargeWork*Found. (Gregory Maxwell) c63198f Make QT runawayException call GetWarnings instead of directly access strMiscWarning. (Gregory Maxwell) --- src/Makefile.am | 2 + src/init.cpp | 1 + src/instantx.cpp | 7 +-- src/qt/dash.cpp | 7 +-- src/timedata.cpp | 5 ++- src/util.cpp | 4 +- src/util.h | 3 +- src/validation.cpp | 97 +++++++--------------------------------- src/validation.h | 1 - src/warnings.cpp | 109 +++++++++++++++++++++++++++++++++++++++++++++ src/warnings.h | 21 +++++++++ 11 files changed, 164 insertions(+), 93 deletions(-) create mode 100644 src/warnings.cpp create mode 100644 src/warnings.h diff --git a/src/Makefile.am b/src/Makefile.am index 03ea731ea..a16aafb1a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -187,6 +187,7 @@ BITCOIN_CORE_H = \ wallet/rpcwallet.h \ wallet/wallet.h \ wallet/walletdb.h \ + warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ zmq/zmqnotificationinterface.h \ @@ -392,6 +393,7 @@ libbitcoin_common_a_SOURCES = \ scheduler.cpp \ script/sign.cpp \ script/standard.cpp \ + warnings.cpp \ $(BITCOIN_CORE_H) # util: shared between all executables. diff --git a/src/init.cpp b/src/init.cpp index c8671fa32..2860ec9d3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -64,6 +64,7 @@ #endif // ENABLE_WALLET #include "privatesend-server.h" #include "spork.h" +#include "warnings.h" #include #include diff --git a/src/instantx.cpp b/src/instantx.cpp index a2144e6ab..b3ea9008e 100644 --- a/src/instantx.cpp +++ b/src/instantx.cpp @@ -17,6 +17,7 @@ #include "util.h" #include "consensus/validation.h" #include "validationinterface.h" +#include "warnings.h" #ifdef ENABLE_WALLET #include "wallet/wallet.h" #endif // ENABLE_WALLET @@ -774,7 +775,7 @@ bool CInstantSend::GetTxLockVote(const uint256& hash, CTxLockVote& txLockVoteRet bool CInstantSend::IsInstantSendReadyToLock(const uint256& txHash) { - if(!fEnableInstantSend || fLargeWorkForkFound || fLargeWorkInvalidChainFound || + if(!fEnableInstantSend || GetfLargeWorkForkFound() || GetfLargeWorkInvalidChainFound() || !sporkManager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED)) return false; LOCK(cs_instantsend); @@ -786,7 +787,7 @@ bool CInstantSend::IsInstantSendReadyToLock(const uint256& txHash) bool CInstantSend::IsLockedInstantSendTransaction(const uint256& txHash) { - if(!fEnableInstantSend || fLargeWorkForkFound || fLargeWorkInvalidChainFound || + if(!fEnableInstantSend || GetfLargeWorkForkFound() || GetfLargeWorkInvalidChainFound() || !sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING)) return false; LOCK(cs_instantsend); @@ -812,7 +813,7 @@ bool CInstantSend::IsLockedInstantSendTransaction(const uint256& txHash) int CInstantSend::GetTransactionLockSignatures(const uint256& txHash) { if(!fEnableInstantSend) return -1; - if(fLargeWorkForkFound || fLargeWorkInvalidChainFound) return -2; + if(GetfLargeWorkForkFound() || GetfLargeWorkInvalidChainFound()) return -2; if(!sporkManager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED)) return -3; LOCK(cs_instantsend); diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index 74de41ebd..f8bb4b3f6 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -33,6 +33,7 @@ #include "scheduler.h" #include "ui_interface.h" #include "util.h" +#include "warnings.h" #ifdef ENABLE_WALLET #include "wallet/wallet.h" @@ -269,7 +270,7 @@ BitcoinCore::BitcoinCore(): void BitcoinCore::handleRunawayException(const std::exception *e) { PrintExceptionContinue(e, "Runaway exception"); - Q_EMIT runawayException(QString::fromStdString(strMiscWarning)); + Q_EMIT runawayException(QString::fromStdString(GetWarnings("gui"))); } void BitcoinCore::initialize() @@ -742,10 +743,10 @@ int main(int argc, char *argv[]) app.exec(); } catch (const std::exception& e) { PrintExceptionContinue(&e, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(strMiscWarning)); + app.handleRunawayException(QString::fromStdString(GetWarnings("gui"))); } catch (...) { PrintExceptionContinue(NULL, "Runaway exception"); - app.handleRunawayException(QString::fromStdString(strMiscWarning)); + app.handleRunawayException(QString::fromStdString(GetWarnings("gui"))); } return app.getReturnValue(); } diff --git a/src/timedata.cpp b/src/timedata.cpp index 774eff51a..406542a52 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -13,6 +13,7 @@ #include "ui_interface.h" #include "util.h" #include "utilstrencodings.h" +#include "warnings.h" #include @@ -103,8 +104,8 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample) if (!fMatch) { fDone = true; - string strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), _(PACKAGE_NAME)); - strMiscWarning = strMessage; + std::string strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), _(PACKAGE_NAME)); + SetMiscWarning(strMessage); uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_WARNING); } } diff --git a/src/util.cpp b/src/util.cpp index 2e4c723a0..0ea7b4eb1 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -124,7 +124,7 @@ map > mapMultiArgs; bool fDebug = false; bool fPrintToConsole = false; bool fPrintToDebugLog = true; -string strMiscWarning; + bool fLogTimestamps = DEFAULT_LOGTIMESTAMPS; bool fLogTimeMicros = DEFAULT_LOGTIMEMICROS; bool fLogThreadNames = DEFAULT_LOGTHREADNAMES; @@ -592,7 +592,7 @@ const boost::filesystem::path &GetBackupsDir() if (fs::is_directory(backupsDir)) return backupsDir; // Fallback to default path if it doesn't LogPrintf("%s: Warning: incorrect parameter -walletbackupsdir, path must exist! Using default path.\n", __func__); - strMiscWarning = _("Warning: incorrect parameter -walletbackupsdir, path must exist! Using default path."); + SetMiscWarning(_("Warning: incorrect parameter -walletbackupsdir, path must exist! Using default path.")); } // Default path backupsDir = GetDataDir() / "backups"; diff --git a/src/util.h b/src/util.h index 5ed2912ea..eb5fb84cb 100644 --- a/src/util.h +++ b/src/util.h @@ -19,6 +19,7 @@ #include "tinyformat.h" #include "utiltime.h" #include "amount.h" +#include "warnings.h" #include #include @@ -66,7 +67,7 @@ extern std::map > mapMultiArgs; extern bool fDebug; extern bool fPrintToConsole; extern bool fPrintToDebugLog; -extern std::string strMiscWarning; + extern bool fLogTimestamps; extern bool fLogTimeMicros; extern bool fLogThreadNames; diff --git a/src/validation.cpp b/src/validation.cpp index ffb913a3a..705d903b8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -37,6 +37,7 @@ #include "utilstrencodings.h" #include "validationinterface.h" #include "versionbits.h" +#include "warnings.h" #include "instantx.h" #include "masternodeman.h" @@ -1327,8 +1328,6 @@ bool IsInitialBlockDownload() return false; } -bool fLargeWorkForkFound = false; -bool fLargeWorkInvalidChainFound = false; CBlockIndex *pindexBestForkTip = NULL, *pindexBestForkBase = NULL; void CheckForkWarningConditions() @@ -1346,7 +1345,7 @@ void CheckForkWarningConditions() if (pindexBestForkTip || (pindexBestInvalid && pindexBestInvalid->nChainWork > chainActive.Tip()->nChainWork + (GetBlockProof(*chainActive.Tip()) * 6))) { - if (!fLargeWorkForkFound && pindexBestForkBase) + if (!GetfLargeWorkForkFound() && pindexBestForkBase) { if(pindexBestForkBase->phashBlock){ std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") + @@ -1360,7 +1359,7 @@ void CheckForkWarningConditions() LogPrintf("%s: Warning: Large valid fork found\n forking the chain at height %d (%s)\n lasting to height %d (%s).\nChain state database corruption likely.\n", __func__, pindexBestForkBase->nHeight, pindexBestForkBase->phashBlock->ToString(), pindexBestForkTip->nHeight, pindexBestForkTip->phashBlock->ToString()); - fLargeWorkForkFound = true; + SetfLargeWorkForkFound(true); } } else @@ -1369,13 +1368,13 @@ void CheckForkWarningConditions() LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__); else LogPrintf("%s: Warning: Found invalid chain which has higher work (at least ~6 blocks worth of work) than our best chain.\nChain state database corruption likely.\n", __func__); - fLargeWorkInvalidChainFound = true; + SetfLargeWorkInvalidChainFound(true); } } else { - fLargeWorkForkFound = false; - fLargeWorkInvalidChainFound = false; + SetfLargeWorkForkFound(false); + SetfLargeWorkInvalidChainFound(false); } } @@ -1645,7 +1644,7 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CDiskBlockPos& pos, const uin /** Abort with a message */ bool AbortNode(const std::string& strMessage, const std::string& userMessage="") { - strMiscWarning = strMessage; + SetMiscWarning(strMessage); LogPrintf("*** %s\n", strMessage); uiInterface.ThreadSafeMessageBox( userMessage.empty() ? _("Error: A fatal internal error occurred, see debug.log for details") : userMessage, @@ -2468,9 +2467,10 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { ThresholdState state = checker.GetStateFor(pindex, chainParams.GetConsensus(), warningcache[bit]); if (state == THRESHOLD_ACTIVE || state == THRESHOLD_LOCKED_IN) { if (state == THRESHOLD_ACTIVE) { - strMiscWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit); + std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit); + SetMiscWarning(strWarning); if (!fWarned) { - CAlert::Notify(strMiscWarning); + CAlert::Notify(strWarning); fWarned = true; } } else { @@ -2490,10 +2490,11 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { warningMessages.push_back(strprintf("%d of last 100 blocks have unexpected version", nUpgraded)); if (nUpgraded > 100/2) { - // strMiscWarning is read by GetWarnings(), called by Qt and the JSON-RPC code to warn the user: - strMiscWarning = _("Warning: Unknown block versions being mined! It's possible unknown rules are in effect"); + std::string strWarning = _("Warning: Unknown block versions being mined! It's possible unknown rules are in effect"); + // notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user: + SetMiscWarning(strWarning); if (!fWarned) { - CAlert::Notify(strMiscWarning); + CAlert::Notify(strWarning); fWarned = true; } } @@ -4400,76 +4401,10 @@ void static CheckBlockIndex(const Consensus::Params& consensusParams) assert(nNodes == forward.size()); } -////////////////////////////////////////////////////////////////////////////// -// -// CAlert -// - -std::string GetWarnings(const std::string& strFor) -{ - int nPriority = 0; - string strStatusBar; - string strRPC; - string strGUI; - const string uiAlertSeperator = "
"; - - if (!CLIENT_VERSION_IS_RELEASE) { - strStatusBar = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"; - strGUI = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"); - } - - if (GetBoolArg("-testsafemode", DEFAULT_TESTSAFEMODE)) - strStatusBar = strRPC = strGUI = "testsafemode enabled"; - - // Misc warnings like out of disk space and clock is wrong - if (strMiscWarning != "") - { - nPriority = 1000; - strStatusBar = strMiscWarning; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning; - } - - if (fLargeWorkForkFound) - { - nPriority = 2000; - strStatusBar = strRPC = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."); - } - else if (fLargeWorkInvalidChainFound) - { - nPriority = 2000; - strStatusBar = strRPC = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."; - strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."); - } - - // Alerts - { - LOCK(cs_mapAlerts); - BOOST_FOREACH(PAIRTYPE(const uint256, CAlert)& item, mapAlerts) - { - const CAlert& alert = item.second; - if (alert.AppliesToMe() && alert.nPriority > nPriority) - { - nPriority = alert.nPriority; - strStatusBar = strGUI = alert.strStatusBar; - } - } - } - - if (strFor == "gui") - return strGUI; - else if (strFor == "statusbar") - return strStatusBar; - else if (strFor == "rpc") - return strRPC; - assert(!"GetWarnings(): invalid parameter"); - return "error"; +std::string CBlockFileInfo::ToString() const { + return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast)); } - std::string CBlockFileInfo::ToString() const { - return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast)); - } - ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos) { AssertLockHeld(cs_main); diff --git a/src/validation.h b/src/validation.h index 290025cf3..d96a47ff5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -134,7 +134,6 @@ static const bool DEFAULT_TIMESTAMPINDEX = false; static const bool DEFAULT_SPENTINDEX = false; static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100; -static const bool DEFAULT_TESTSAFEMODE = false; /** Default for -mempoolreplacement */ static const bool DEFAULT_ENABLE_REPLACEMENT = false; /** Default for using fee filter */ diff --git a/src/warnings.cpp b/src/warnings.cpp new file mode 100644 index 000000000..97329e75c --- /dev/null +++ b/src/warnings.cpp @@ -0,0 +1,109 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "sync.h" +#include "clientversion.h" +#include "util.h" +#include "warnings.h" +#include "alert.h" +#include "hash.h" + +CCriticalSection cs_warnings; +std::string strMiscWarning; +bool fLargeWorkForkFound = false; +bool fLargeWorkInvalidChainFound = false; + +void SetMiscWarning(const std::string& strWarning) +{ + LOCK(cs_warnings); + strMiscWarning = strWarning; +} + +void SetfLargeWorkForkFound(bool flag) +{ + LOCK(cs_warnings); + fLargeWorkForkFound = flag; +} + +bool GetfLargeWorkForkFound() +{ + LOCK(cs_warnings); + return fLargeWorkForkFound; +} + +void SetfLargeWorkInvalidChainFound(bool flag) +{ + LOCK(cs_warnings); + fLargeWorkInvalidChainFound = flag; +} + +bool GetfLargeWorkInvalidChainFound() +{ + LOCK(cs_warnings); + return fLargeWorkInvalidChainFound; +} + +std::string GetWarnings(const std::string& strFor) +{ + int nPriority = 0; + std::string strStatusBar; + std::string strRPC; + std::string strGUI; + const std::string uiAlertSeperator = "
"; + + LOCK(cs_warnings); + + if (!CLIENT_VERSION_IS_RELEASE) { + strStatusBar = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"; + strGUI = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"); + } + + if (GetBoolArg("-testsafemode", DEFAULT_TESTSAFEMODE)) + strStatusBar = strRPC = strGUI = "testsafemode enabled"; + + // Misc warnings like out of disk space and clock is wrong + if (strMiscWarning != "") + { + nPriority = 1000; + strStatusBar = strMiscWarning; + strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning; + } + + if (fLargeWorkForkFound) + { + nPriority = 2000; + strStatusBar = strRPC = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."; + strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues."); + } + else if (fLargeWorkInvalidChainFound) + { + nPriority = 2000; + strStatusBar = strRPC = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."; + strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."); + } + + // Alerts + { + LOCK(cs_mapAlerts); + for(auto& item : mapAlerts) + { + const CAlert& alert = item.second; + if (alert.AppliesToMe() && alert.nPriority > nPriority) + { + nPriority = alert.nPriority; + strStatusBar = strGUI = alert.strStatusBar; + } + } + } + + if (strFor == "gui") + return strGUI; + else if (strFor == "statusbar") + return strStatusBar; + else if (strFor == "rpc") + return strRPC; + assert(!"GetWarnings(): invalid parameter"); + return "error"; +} diff --git a/src/warnings.h b/src/warnings.h new file mode 100644 index 000000000..a7aa65742 --- /dev/null +++ b/src/warnings.h @@ -0,0 +1,21 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WARNINGS_H +#define BITCOIN_WARNINGS_H + +#include +#include + +void SetMiscWarning(const std::string& strWarning); +void SetfLargeWorkForkFound(bool flag); +bool GetfLargeWorkForkFound(); +void SetfLargeWorkInvalidChainFound(bool flag); +bool GetfLargeWorkInvalidChainFound(); +std::string GetWarnings(const std::string& strFor); + +static const bool DEFAULT_TESTSAFEMODE = false; + +#endif // BITCOIN_WARNINGS_H