diff options
author | Jesse Cohen <jc@jc.lol> | 2018-05-10 15:57:16 -0400 |
---|---|---|
committer | Jesse Cohen <jc@jc.lol> | 2018-05-12 12:44:32 -0400 |
commit | a3ae8e68739023e5dba9e5cb190e707ed4603316 (patch) | |
tree | 6995c621267b759e46cbda4b9141583a606dd765 | |
parent | ecc3c4a019e6db30e208b8554b1a3658dcb9a80a (diff) | |
download | bitcoin-a3ae8e68739023e5dba9e5cb190e707ed4603316.tar.xz |
Fix concurrency-related bugs in ActivateBestChain
If multiple threads are invoking ActivateBestChain, it was possible to have
them working towards different tips, and we could arrive at a less work tip
than we should. Fix this by introducing a ChainState lock which must
be held for the entire duration of ActivateBestChain to enforce
exclusion in ABC.
-rw-r--r-- | src/validation.cpp | 13 |
1 files changed, 13 insertions, 0 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index 477b3e1550..405f21baa3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -144,6 +144,12 @@ private: */ std::set<CBlockIndex*> g_failed_blocks; + /** + * the ChainState CriticalSection + * A lock that must be held when modifying this ChainState - held in ActivateBestChain() + */ + CCriticalSection m_cs_chainstate; + public: CChain chainActive; BlockMap mapBlockIndex; @@ -2542,6 +2548,7 @@ void CChainState::PruneBlockIndexCandidates() { bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); + const CBlockIndex *pindexOldTip = chainActive.Tip(); const CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); @@ -2653,6 +2660,12 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& // sanely for performance or correctness! AssertLockNotHeld(cs_main); + // ABC maintains a fair degree of expensive-to-calculate internal state + // because this function periodically releases cs_main so that it does not lock up other threads for too long + // during large connects - and to allow for e.g. the callback queue to drain + // we use m_cs_chainstate to enforce mutual exclusion so that only one caller may execute this function at a time + LOCK(m_cs_chainstate); + CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; int nStopAtHeight = gArgs.GetArg("-stopatheight", DEFAULT_STOPATHEIGHT); |