Eliminate data races for strMiscWarning and fLargeWork*Found.

furszy · furszy · commit e074feb3e563 · 2020-12-14T20:34:27.000-03:00
This moves all access to these datastructures through accessor functions and protects them with a lock.

Coming from btc@e3ba0ef95636290a3bb597ddd25d13ea13b034aa
diff --git a/src/timedata.cpp b/src/timedata.cpp
@@ -74,11 +74,11 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample, int nOffsetLimit)
         // Only let other nodes change our time by so much
         if (abs64(nMedian) < nOffsetLimit) {
             nTimeOffset = nMedian;
-            strMiscWarning = "";
+            SetMiscWarning("");
         } else {
             nTimeOffset = (nMedian > 0 ? 1 : -1) * nOffsetLimit;
             std::string strMessage = _("Warning: Please check that your computer's date and time are correct! If your clock is wrong PIVX Core will not work properly.");
-            strMiscWarning = strMessage;
+            SetMiscWarning(strMessage);
             LogPrintf("*** %s\n", strMessage);
             uiInterface.ThreadSafeMessageBox(strMessage, "", CClientUIInterface::MSG_ERROR);
         }
diff --git a/src/util.cpp b/src/util.cpp
@@ -100,6 +100,7 @@ ArgsManager gArgs;
 bool fDaemon = false;
 CTranslationInterface translationInterface;
 
+RecursiveMutex cs_warnings;
 std::string strMiscWarning;
 bool fLargeWorkForkFound = false;
 bool fLargeWorkInvalidChainFound = false;
@@ -843,11 +844,43 @@ int GetNumCores()
     return std::thread::hardware_concurrency();
 }
 
+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)
 {
     std::string strStatusBar;
     std::string strRPC;
 
+    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 staking or merchant applications!");
 
diff --git a/src/util.h b/src/util.h
@@ -58,9 +58,6 @@ extern std::string strBudgetMode;
 extern CTranslationInterface translationInterface;
 
 static const bool DEFAULT_TESTSAFEMODE = false;
-extern std::string strMiscWarning;
-extern bool fLargeWorkForkFound;
-extern bool fLargeWorkInvalidChainFound;
 
 /**
  * Translation function: Call Translate signal on UI interface, which returns a Optional result.
@@ -266,6 +263,11 @@ void TraceThread(const char* name, Callable func)
 
 fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific = true);
 
+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
@@ -874,7 +874,7 @@ void CheckForkWarningConditions()
         pindexBestForkTip = nullptr;
 
     if (pindexBestForkTip || (pindexBestInvalid && pindexBestInvalid->nChainWork > pChainTip->nChainWork + (GetBlockProof(*pChainTip) * 6))) {
-        if (!fLargeWorkForkFound && pindexBestForkBase) {
+        if (!GetfLargeWorkForkFound() && pindexBestForkBase) {
             if (pindexBestForkBase->phashBlock) {
                 std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") +
                                       pindexBestForkBase->phashBlock->ToString() + std::string("'");
@@ -886,15 +886,15 @@ void CheckForkWarningConditions()
                 LogPrintf("CheckForkWarningConditions: 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",
                     pindexBestForkBase->nHeight, pindexBestForkBase->phashBlock->ToString(),
                     pindexBestForkTip->nHeight, pindexBestForkTip->phashBlock->ToString());
-                fLargeWorkForkFound = true;
+                SetfLargeWorkForkFound(true);
             }
         } else {
             LogPrintf("CheckForkWarningConditions: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n");
-            fLargeWorkInvalidChainFound = true;
+            SetfLargeWorkInvalidChainFound(true);
         }
     } else {
-        fLargeWorkForkFound = false;
-        fLargeWorkInvalidChainFound = false;
+        SetfLargeWorkForkFound(false);
+        SetfLargeWorkInvalidChainFound(false);
     }
 }
 
@@ -1184,7 +1184,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
 /** Abort with a message */
 static 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 occured, see debug.log for details") : userMessage,
@@ -1922,10 +1922,12 @@ void static UpdateTip(CBlockIndex* pindexNew)
         if (nUpgraded > 0)
             LogPrintf("SetBestChain: %d of last 100 blocks above version %d\n", nUpgraded, (int)CBlock::CURRENT_VERSION);
         if (nUpgraded > 100 / 2) {
-            // strMiscWarning is read by GetWarnings(), called by Qt and the JSON-RPC code to warn the user:
-            strMiscWarning = _("Warning: This version is obsolete, upgrade required!");
-            AlertNotify(strMiscWarning);
-            fWarned = true;
+            std::string strWarning = _("Warning: This version is obsolete, upgrade required!");
+            SetMiscWarning(strWarning);
+            if (!fWarned) {
+                AlertNotify(strWarning);
+                fWarned = true;
+            }
         }
     }
 }
