From f262a8e85ef585e90d95bd939605a3f52296e223 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 5 Oct 2017 15:03:09 +0200 Subject: [PATCH] Merge #11107: Fix races in AppInitMain and others with lock and atomic bools c626dcb50 Make fUseCrypto atomic (MeshCollider) 731065b11 Consistent parameter names in txdb.h (MeshCollider) 35aeabec6 Make fReindex atomic to avoid race (MeshCollider) 58d91af59 Fix race for mapBlockIndex in AppInitMain (MeshCollider) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/11106 Also makes fReindex atomic as suggested in @TheBlueMatt comment below, and makes fUseCrypto atomic as suggested in 10916 https://github.com/bitcoin/bitcoin/pull/11107/commits/d291e7635b0ef4156c2805c6c4ee1adad91f0307 just renames the parameters in the txdb header file to make them consistent with those used in the cpp file, noticed it when looking for uses of fReindex Tree-SHA512: b378aa7289fd505b76565cd4d48dcdc04ac5540283ea1c80442170b0f13cb6df771b1a94dd54b7fec3478a7b4668c224ec9d795f16937782724c5d020edd3a42 --- src/init.cpp | 12 +++++++++--- src/txdb.h | 8 ++++---- src/validation.cpp | 4 ++-- src/validation.h | 2 +- src/wallet/crypter.h | 4 +++- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 8a50fd7259cde8..978bfb2f2eb4db 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2189,9 +2189,15 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 12: start node + int chain_active_height; + //// debug print - LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); - LogPrintf("chainActive.Height() = %d\n", chainActive.Height()); + { + LOCK(cs_main); + LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); + chain_active_height = chainActive.Height(); + } + LogPrintf("chainActive.Height() = %d\n", chain_active_height); if (gArgs.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION)) StartTorControl(threadGroup, scheduler); @@ -2206,7 +2212,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.m_msgproc = peerLogic.get(); connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER); diff --git a/src/txdb.h b/src/txdb.h index 092204668fbb86..4f8019d3ed6b51 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -117,13 +117,13 @@ class CBlockTreeDB : public CDBWrapper CBlockTreeDB& operator=(const CBlockTreeDB&) = delete; bool WriteBatchSync(const std::vector >& fileInfo, int nLastFile, const std::vector& 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 HasTxIndex(const uint256 &txid); bool ReadTxIndex(const uint256 &txid, CDiskTxPos &pos); - bool WriteTxIndex(const std::vector > &list); + bool WriteTxIndex(const std::vector > &vect); bool ReadSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value); bool UpdateSpentIndex(const std::vector >&vect); bool UpdateAddressUnspentIndex(const std::vector >&vect); diff --git a/src/validation.cpp b/src/validation.cpp index faf1850176230d..edb65e9c6307a2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -81,7 +81,7 @@ CWaitableCriticalSection csBestBlock; CConditionVariable cvBlockChange; int nScriptCheckThreads = 0; std::atomic_bool fImporting(false); -bool fReindex = false; +std::atomic_bool fReindex(false); bool fTxIndex = true; bool fAddressIndex = false; bool fTimestampIndex = false; @@ -3968,7 +3968,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) // Check whether we need to continue reindexing bool fReindexing = false; pblocktree->ReadReindexing(fReindexing); - fReindex |= fReindexing; + if(fReindexing) fReindex = true; // Check whether we have a transaction index pblocktree->ReadFlag("txindex", fTxIndex); diff --git a/src/validation.h b/src/validation.h index 9348873fc3fdb1..d9bb9686e6d08f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -170,7 +170,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 fAddressIndex; diff --git a/src/wallet/crypter.h b/src/wallet/crypter.h index 18ecdd71dfbd5d..71ddfa73d64900 100644 --- a/src/wallet/crypter.h +++ b/src/wallet/crypter.h @@ -9,6 +9,8 @@ #include "serialize.h" #include "support/allocators/secure.h" +#include + const unsigned int WALLET_CRYPTO_KEY_SIZE = 32; const unsigned int WALLET_CRYPTO_SALT_SIZE = 8; const unsigned int WALLET_CRYPTO_IV_SIZE = 16; @@ -124,7 +126,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 fUseCrypto; //! keeps track of whether Unlock has run a thorough check before bool fDecryptionThoroughlyChecked;