From 02de6a6bcda8bef4c35eb6c5135529fdf51f0f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Thu, 11 Jan 2018 23:32:32 +0000 Subject: Assert cs_main is held when accessing mapBlockIndex --- src/validation.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/validation.cpp') diff --git a/src/validation.cpp b/src/validation.cpp index a77362f5d..18412d9e9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -260,6 +260,8 @@ namespace { CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { + AssertLockHeld(cs_main); + // Find the first block the caller has in the main chain for (const uint256& hash : locator.vHave) { BlockMap::iterator mi = mapBlockIndex.find(hash); @@ -2774,6 +2776,8 @@ bool ResetBlockFailureFlags(CBlockIndex *pindex) { CBlockIndex* CChainState::AddToBlockIndex(const CBlockHeader& block) { + AssertLockHeld(cs_main); + // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator it = mapBlockIndex.find(hash); @@ -3654,6 +3658,8 @@ fs::path GetBlockPosFilename(const CDiskBlockPos &pos, const char *prefix) CBlockIndex * CChainState::InsertBlockIndex(const uint256& hash) { + AssertLockHeld(cs_main); + if (hash.IsNull()) return nullptr; @@ -3781,6 +3787,8 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) bool LoadChainTip(const CChainParams& chainparams) { + AssertLockHeld(cs_main); + if (chainActive.Tip() && chainActive.Tip()->GetBlockHash() == pcoinsTip->GetBestBlock()) return true; if (pcoinsTip->GetBestBlock().IsNull() && mapBlockIndex.size() == 1) { -- cgit v1.2.3 From f814a3e8fa0d99c3d95ae7866c707617a1dd3d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Thu, 11 Jan 2018 23:40:43 +0000 Subject: Fix cs_main lock in LoadExternalBlockFile When accessing mapBlockIndex cs_main must be held. --- src/validation.cpp | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) (limited to 'src/validation.cpp') diff --git a/src/validation.cpp b/src/validation.cpp index 18412d9e9..5ccbb49ac 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4252,26 +4252,28 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB blkdat >> block; nRewind = blkdat.GetPos(); - // detect out of order blocks, and store them for later uint256 hash = block.GetHash(); - if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex.find(block.hashPrevBlock) == mapBlockIndex.end()) { - LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), - block.hashPrevBlock.ToString()); - if (dbp) - mapBlocksUnknownParent.insert(std::make_pair(block.hashPrevBlock, *dbp)); - continue; - } - - // process in case the block isn't known yet - if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { + { LOCK(cs_main); - CValidationState state; - if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) - nLoaded++; - if (state.IsError()) - break; - } else if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex[hash]->nHeight % 1000 == 0) { - LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), mapBlockIndex[hash]->nHeight); + // detect out of order blocks, and store them for later + if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex.find(block.hashPrevBlock) == mapBlockIndex.end()) { + LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), + block.hashPrevBlock.ToString()); + if (dbp) + mapBlocksUnknownParent.insert(std::make_pair(block.hashPrevBlock, *dbp)); + continue; + } + + // process in case the block isn't known yet + if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { + CValidationState state; + if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) + nLoaded++; + if (state.IsError()) + break; + } else if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex[hash]->nHeight % 1000 == 0) { + LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), mapBlockIndex[hash]->nHeight); + } } // Activate the genesis block so normal node progress can continue -- cgit v1.2.3 From 92fabcd443322dcfdf2b3477515fae79e8647d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Fri, 12 Jan 2018 00:23:09 +0000 Subject: Add LookupBlockIndex function --- src/validation.cpp | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'src/validation.cpp') diff --git a/src/validation.cpp b/src/validation.cpp index 5ccbb49ac..491f311b9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -264,10 +264,8 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc // Find the first block the caller has in the main chain for (const uint256& hash : locator.vHave) { - BlockMap::iterator mi = mapBlockIndex.find(hash); - if (mi != mapBlockIndex.end()) - { - CBlockIndex* pindex = (*mi).second; + CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex) { if (chain.Contains(pindex)) return pindex; if (pindex->GetAncestor(chain.Height()) == chain.Tip()) { @@ -1319,7 +1317,7 @@ bool CScriptCheck::operator()() { int GetSpendHeight(const CCoinsViewCache& inputs) { LOCK(cs_main); - CBlockIndex* pindexPrev = mapBlockIndex.find(inputs.GetBestBlock())->second; + CBlockIndex* pindexPrev = LookupBlockIndex(inputs.GetBestBlock()); return pindexPrev->nHeight + 1; } @@ -3224,7 +3222,6 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& BlockMap::iterator miSelf = mapBlockIndex.find(hash); CBlockIndex *pindex = nullptr; if (hash != chainparams.GetConsensus().hashGenesisBlock) { - if (miSelf != mapBlockIndex.end()) { // Block header is already known. pindex = miSelf->second; @@ -3802,10 +3799,11 @@ bool LoadChainTip(const CChainParams& chainparams) } // Load pointer to end of best chain - BlockMap::iterator it = mapBlockIndex.find(pcoinsTip->GetBestBlock()); - if (it == mapBlockIndex.end()) + CBlockIndex* pindex = LookupBlockIndex(pcoinsTip->GetBestBlock()); + if (!pindex) { return false; - chainActive.SetTip(it->second); + } + chainActive.SetTip(pindex); g_chainstate.PruneBlockIndexCandidates(); @@ -4256,7 +4254,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB { LOCK(cs_main); // detect out of order blocks, and store them for later - if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex.find(block.hashPrevBlock) == mapBlockIndex.end()) { + if (hash != chainparams.GetConsensus().hashGenesisBlock && !LookupBlockIndex(block.hashPrevBlock)) { LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), block.hashPrevBlock.ToString()); if (dbp) @@ -4265,14 +4263,17 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB } // process in case the block isn't known yet - if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { - CValidationState state; - if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) - nLoaded++; - if (state.IsError()) - break; - } else if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex[hash]->nHeight % 1000 == 0) { - LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), mapBlockIndex[hash]->nHeight); + CBlockIndex* pindex = LookupBlockIndex(hash); + if (!pindex || (pindex->nStatus & BLOCK_HAVE_DATA) == 0) { + CValidationState state; + if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) { + nLoaded++; + } + if (state.IsError()) { + break; + } + } else if (hash != chainparams.GetConsensus().hashGenesisBlock && pindex->nHeight % 1000 == 0) { + LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), pindex->nHeight); } } -- cgit v1.2.3