Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix races in AppInitMain and others with lock and atomic bools #11107

Merged
merged 4 commits into from Oct 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/init.cpp
Expand Up @@ -1631,9 +1631,16 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)

// ********************************************************* Step 11: start node

int chain_active_height;

//// debug print
LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size());
LogPrintf("nBestHeight = %d\n", chainActive.Height());
{
LOCK(cs_main);
LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size());
chain_active_height = chainActive.Height();
}
LogPrintf("nBestHeight = %d\n", chain_active_height);

if (gArgs.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION))
StartTorControl(threadGroup, scheduler);

Expand All @@ -1649,7 +1656,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections);
connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS;
connOptions.nMaxFeeler = 1;
connOptions.nBestHeight = chainActive.Height();
connOptions.nBestHeight = chain_active_height;
connOptions.uiInterface = &uiInterface;
connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER);
connOptions.nReceiveFloodSize = 1000*gArgs.GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER);
Expand Down
8 changes: 4 additions & 4 deletions src/txdb.h
Expand Up @@ -115,12 +115,12 @@ class CBlockTreeDB : public CDBWrapper
void operator=(const CBlockTreeDB&);
public:
bool WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*> >& fileInfo, int nLastFile, const std::vector<const CBlockIndex*>& blockinfo);
bool ReadBlockFileInfo(int nFile, CBlockFileInfo &fileinfo);
bool ReadBlockFileInfo(int nFile, CBlockFileInfo &info);
bool ReadLastBlockFile(int &nFile);
bool WriteReindexing(bool fReindex);
bool ReadReindexing(bool &fReindex);
bool WriteReindexing(bool fReindexing);
bool ReadReindexing(bool &fReindexing);
bool ReadTxIndex(const uint256 &txid, CDiskTxPos &pos);
bool WriteTxIndex(const std::vector<std::pair<uint256, CDiskTxPos> > &list);
bool WriteTxIndex(const std::vector<std::pair<uint256, CDiskTxPos> > &vect);
bool WriteFlag(const std::string &name, bool fValue);
bool ReadFlag(const std::string &name, bool &fValue);
bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex);
Expand Down
4 changes: 2 additions & 2 deletions src/validation.cpp
Expand Up @@ -66,7 +66,7 @@ CWaitableCriticalSection csBestBlock;
CConditionVariable cvBlockChange;
int nScriptCheckThreads = 0;
std::atomic_bool fImporting(false);
bool fReindex = false;
std::atomic_bool fReindex(false);
bool fTxIndex = false;
bool fHavePruned = false;
bool fPruneMode = false;
Expand Down Expand Up @@ -3523,7 +3523,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams)
// Check whether we need to continue reindexing
bool fReindexing = false;
pblocktree->ReadReindexing(fReindexing);
fReindex |= fReindexing;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with |= ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my reply to @TheBlueMatt's comment above, it's not implemented for atomic bool :)

if(fReindexing) fReindex = true;

// Check whether we have a transaction index
pblocktree->ReadFlag("txindex", fTxIndex);
Expand Down
2 changes: 1 addition & 1 deletion src/validation.h
Expand Up @@ -167,7 +167,7 @@ extern const std::string strMessageMagic;
extern CWaitableCriticalSection csBestBlock;
extern CConditionVariable cvBlockChange;
extern std::atomic_bool fImporting;
extern bool fReindex;
extern std::atomic_bool fReindex;
extern int nScriptCheckThreads;
extern bool fTxIndex;
extern bool fIsBareMultisigStd;
Expand Down
4 changes: 3 additions & 1 deletion src/wallet/crypter.h
Expand Up @@ -9,6 +9,8 @@
#include "serialize.h"
#include "support/allocators/secure.h"

#include <atomic>

const unsigned int WALLET_CRYPTO_KEY_SIZE = 32;
const unsigned int WALLET_CRYPTO_SALT_SIZE = 8;
const unsigned int WALLET_CRYPTO_IV_SIZE = 16;
Expand Down Expand Up @@ -118,7 +120,7 @@ class CCryptoKeyStore : public CBasicKeyStore

//! if fUseCrypto is true, mapKeys must be empty
//! if fUseCrypto is false, vMasterKey must be empty
bool fUseCrypto;
std::atomic<bool> fUseCrypto;

//! keeps track of whether Unlock has run a thorough check before
bool fDecryptionThoroughlyChecked;
Expand Down