Skip to content

Commit

Permalink
Merge #9236: Fix races for strMiscWarning and fLargeWork*Found, make …
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
laanwj committed Dec 19, 2016
2 parents a336d13 + 749be01 commit 7f72568
Show file tree
Hide file tree
Showing 10 changed files with 138 additions and 64 deletions.
2 changes: 2 additions & 0 deletions src/Makefile.am
Expand Up @@ -155,6 +155,7 @@ BITCOIN_CORE_H = \
wallet/rpcwallet.h \
wallet/wallet.h \
wallet/walletdb.h \
warnings.h \
zmq/zmqabstractnotifier.h \
zmq/zmqconfig.h\
zmq/zmqnotificationinterface.h \
Expand Down Expand Up @@ -306,6 +307,7 @@ libbitcoin_common_a_SOURCES = \
scheduler.cpp \
script/sign.cpp \
script/standard.cpp \
warnings.cpp \
$(BITCOIN_CORE_H)

# util: shared between all executables.
Expand Down
1 change: 1 addition & 0 deletions src/init.cpp
Expand Up @@ -41,6 +41,7 @@
#ifdef ENABLE_WALLET
#include "wallet/wallet.h"
#endif
#include "warnings.h"
#include <stdint.h>
#include <stdio.h>
#include <memory>
Expand Down
7 changes: 4 additions & 3 deletions src/qt/bitcoin.cpp
Expand Up @@ -30,6 +30,7 @@
#include "scheduler.h"
#include "ui_interface.h"
#include "util.h"
#include "warnings.h"

#ifdef ENABLE_WALLET
#include "wallet/wallet.h"
Expand Down Expand Up @@ -260,7 +261,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()
Expand Down Expand Up @@ -691,10 +692,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();
}
Expand Down
5 changes: 3 additions & 2 deletions src/timedata.cpp
Expand Up @@ -13,6 +13,7 @@
#include "ui_interface.h"
#include "util.h"
#include "utilstrencodings.h"
#include "warnings.h"

#include <boost/foreach.hpp>

Expand Down Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/util.cpp
Expand Up @@ -107,7 +107,7 @@ map<string, vector<string> > mapMultiArgs;
bool fDebug = false;
bool fPrintToConsole = false;
bool fPrintToDebugLog = true;
string strMiscWarning;

bool fLogTimestamps = DEFAULT_LOGTIMESTAMPS;
bool fLogTimeMicros = DEFAULT_LOGTIMEMICROS;
bool fLogIPs = DEFAULT_LOGIPS;
Expand Down
2 changes: 1 addition & 1 deletion src/util.h
Expand Up @@ -46,7 +46,7 @@ extern std::map<std::string, std::vector<std::string> > mapMultiArgs;
extern bool fDebug;
extern bool fPrintToConsole;
extern bool fPrintToDebugLog;
extern std::string strMiscWarning;

extern bool fLogTimestamps;
extern bool fLogTimeMicros;
extern bool fLogIPs;
Expand Down
72 changes: 16 additions & 56 deletions src/validation.cpp
Expand Up @@ -34,6 +34,7 @@
#include "utilstrencodings.h"
#include "validationinterface.h"
#include "versionbits.h"
#include "warnings.h"

#include <atomic>
#include <sstream>
Expand Down Expand Up @@ -1141,8 +1142,6 @@ bool IsInitialBlockDownload()
return false;
}

bool fLargeWorkForkFound = false;
bool fLargeWorkInvalidChainFound = false;
CBlockIndex *pindexBestForkTip = NULL, *pindexBestForkBase = NULL;

static void AlertNotify(const std::string& strMessage)
Expand Down Expand Up @@ -1177,7 +1176,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("'");
Expand All @@ -1188,18 +1187,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);
}
}

Expand Down Expand Up @@ -1475,7 +1474,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,
Expand Down Expand Up @@ -2044,9 +2043,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 {
Expand All @@ -2066,10 +2066,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;
}
}
Expand Down Expand Up @@ -4010,51 +4011,10 @@ void static CheckBlockIndex(const Consensus::Params& consensusParams)
assert(nNodes == forward.size());
}

std::string GetWarnings(const std::string& strFor)
std::string CBlockFileInfo::ToString() const
{
string strStatusBar;
string strRPC;
string strGUI;
const string uiAlertSeperator = "<hr />";

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 != "")
{
strStatusBar = strMiscWarning;
strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning;
}

if (fLargeWorkForkFound)
{
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)
{
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.");
}

if (strFor == "gui")
return strGUI;
else if (strFor == "statusbar")
return strStatusBar;
else if (strFor == "rpc")
return strRPC;
assert(!"GetWarnings(): invalid parameter");
return "error";
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)
{
Expand Down
1 change: 0 additions & 1 deletion src/validation.h
Expand Up @@ -137,7 +137,6 @@ static const bool DEFAULT_CHECKPOINTS_ENABLED = true;
static const bool DEFAULT_TXINDEX = false;
static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100;

static const bool DEFAULT_TESTSAFEMODE = false;
/** Default for -mempoolreplacement */
static const bool DEFAULT_ENABLE_REPLACEMENT = true;
/** Default for using fee filter */
Expand Down
89 changes: 89 additions & 0 deletions src/warnings.cpp
@@ -0,0 +1,89 @@
// 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"

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)
{
std::string strStatusBar;
std::string strRPC;
std::string strGUI;
const std::string uiAlertSeperator = "<hr />";

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 != "")
{
strStatusBar = strMiscWarning;
strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + strMiscWarning;
}

if (fLargeWorkForkFound)
{
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)
{
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.");
}

if (strFor == "gui")
return strGUI;
else if (strFor == "statusbar")
return strStatusBar;
else if (strFor == "rpc")
return strRPC;
assert(!"GetWarnings(): invalid parameter");
return "error";
}
21 changes: 21 additions & 0 deletions 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 <stdlib.h>
#include <string>

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

0 comments on commit 7f72568

Please sign in to comment.