From 1dfd31bc267c54144a7e62ad5a1a5860c032f4d7 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Wed, 19 Jan 2022 14:43:15 -0300 Subject: [PATCH 1/3] scripted-diff: rename m_cs_chainstate -> m_chainstate_mutex -BEGIN VERIFY SCRIPT- s() { sed -i 's/m_cs_chainstate/m_chainstate_mutex/g' $1; } s src/validation.cpp s src/validation.h -END VERIFY SCRIPT- --- src/validation.cpp | 6 +++--- src/validation.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index c521e9b634..7c0654c2c6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2830,8 +2830,8 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr // 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); + // we use m_chainstate_mutex to enforce mutual exclusion so that only one caller may execute this function at a time + LOCK(m_chainstate_mutex); CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; @@ -2961,7 +2961,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind // We do not allow ActivateBestChain() to run while InvalidateBlock() is // running, as that could cause the tip to change while we disconnect // blocks. - LOCK(m_cs_chainstate); + LOCK(m_chainstate_mutex); // We'll be acquiring and releasing cs_main below, to allow the validation // callbacks to run. However, we should keep the block index in a diff --git a/src/validation.h b/src/validation.h index 160fffd048..ae3749bb47 100644 --- a/src/validation.h +++ b/src/validation.h @@ -536,7 +536,7 @@ protected: * the ChainState CriticalSection * A lock that must be held when modifying this ChainState - held in ActivateBestChain() */ - RecursiveMutex m_cs_chainstate; + RecursiveMutex m_chainstate_mutex; /** * Whether this chainstate is undergoing initial block download. From ddeefeef20fa2fe48c3c4563370a6297704d228e Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Wed, 19 Jan 2022 18:47:54 -0300 Subject: [PATCH 2/3] refactor: add negative TS annotations for `m_chainstate_mutex` Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/validation.cpp | 4 ++++ src/validation.h | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 7c0654c2c6..bc69bc4230 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2821,6 +2821,8 @@ static void LimitValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main) { bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr pblock) { + AssertLockNotHeld(m_chainstate_mutex); + // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set @@ -2950,6 +2952,8 @@ bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex) { + AssertLockNotHeld(m_chainstate_mutex); + // Genesis block can't be invalidated assert(pindex); if (pindex->nHeight == 0) return false; diff --git a/src/validation.h b/src/validation.h index ae3749bb47..d3c43c26e5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -700,7 +700,7 @@ public: */ bool ActivateBestChain( BlockValidationState& state, - std::shared_ptr pblock = nullptr) LOCKS_EXCLUDED(cs_main); + std::shared_ptr pblock = nullptr) LOCKS_EXCLUDED(m_chainstate_mutex, cs_main); bool AcceptBlock(const std::shared_ptr& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -720,7 +720,7 @@ public: */ bool PreciousBlock(BlockValidationState& state, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); /** Mark a block as invalid. */ - bool InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); + bool InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex) LOCKS_EXCLUDED(m_chainstate_mutex, cs_main); /** Remove invalidity status from a block and its descendants. */ void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); From 020acea99b605c9b5ee7939a6acef131db84ad4a Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Wed, 19 Jan 2022 14:45:49 -0300 Subject: [PATCH 3/3] refactor: replace RecursiveMutex m_chainstate_mutex with Mutex --- src/validation.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/validation.h b/src/validation.h index d3c43c26e5..21b8f9579c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -533,10 +533,11 @@ protected: arith_uint256 nLastPreciousChainwork = 0; /** - * the ChainState CriticalSection - * A lock that must be held when modifying this ChainState - held in ActivateBestChain() + * The ChainState Mutex + * A lock that must be held when modifying this ChainState - held in ActivateBestChain() and + * InvalidateBlock() */ - RecursiveMutex m_chainstate_mutex; + Mutex m_chainstate_mutex; /** * Whether this chainstate is undergoing initial block download.