From e3ba0ef95636290a3bb597ddd25d13ea13b034aa Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Tue, 29 Nov 2016 09:46:19 +0000 Subject: [PATCH] Eliminate data races for strMiscWarning and fLargeWork*Found. This moves all access to these datastructures through accessor functions and protects them with a lock. --- src/timedata.cpp | 4 ++-- src/util.cpp | 33 +++++++++++++++++++++++++++++++++ src/util.h | 8 +++++--- src/validation.cpp | 24 +++++++++++++----------- 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/timedata.cpp b/src/timedata.cpp index 25fc49412..256c91016 100644 --- a/src/timedata.cpp +++ b/src/timedata.cpp @@ -103,8 +103,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 a2e6b85d2..92a5b34a3 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -108,6 +108,7 @@ bool fDebug = false; bool fPrintToConsole = false; bool fPrintToDebugLog = true; +CCriticalSection cs_warnings; string strMiscWarning; bool fLargeWorkForkFound = false; bool fLargeWorkInvalidChainFound = false; @@ -813,6 +814,36 @@ std::string CopyrightHolders(const std::string& strPrefix) return strCopyrightHolders; } +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) { string strStatusBar; @@ -820,6 +851,8 @@ std::string GetWarnings(const std::string& strFor) string strGUI; const 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"); diff --git a/src/util.h b/src/util.h index edc190e1e..b34966fd8 100644 --- a/src/util.h +++ b/src/util.h @@ -48,9 +48,6 @@ extern bool fPrintToConsole; extern bool fPrintToDebugLog; static const bool DEFAULT_TESTSAFEMODE = false; -extern std::string strMiscWarning; -extern bool fLargeWorkForkFound; -extern bool fLargeWorkInvalidChainFound; extern bool fLogTimestamps; extern bool fLogTimeMicros; @@ -229,6 +226,11 @@ template void TraceThread(const char* name, Callable func) std::string CopyrightHolders(const std::string& strPrefix); +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); #endif // BITCOIN_UTIL_H diff --git a/src/validation.cpp b/src/validation.cpp index a17ae041d..fde52ea74 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1183,7 +1183,7 @@ void CheckForkWarningConditions() if (pindexBestForkTip || (pindexBestInvalid && pindexBestInvalid->nChainWork > chainActive.Tip()->nChainWork + (GetBlockProof(*chainActive.Tip()) * 6))) { - if (!fLargeWorkForkFound && pindexBestForkBase) + if (!GetfLargeWorkForkFound() && pindexBestForkBase) { std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") + pindexBestForkBase->phashBlock->ToString() + std::string("'"); @@ -1194,18 +1194,18 @@ 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 { LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__); - fLargeWorkInvalidChainFound = true; + SetfLargeWorkInvalidChainFound(true); } } else { - fLargeWorkForkFound = false; - fLargeWorkInvalidChainFound = false; + SetfLargeWorkForkFound(false); + SetfLargeWorkInvalidChainFound(false); } } @@ -1481,7 +1481,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, @@ -2050,9 +2050,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) { - AlertNotify(strMiscWarning); + AlertNotify(strWarning); fWarned = true; } } else { @@ -2072,10 +2073,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) { - AlertNotify(strMiscWarning); + AlertNotify(strWarning); fWarned = true; } }