Merge bitcoin#24103: Replace RecursiveMutex m_cs_chainstate with Mutex, and rename it

020acea refactor: replace RecursiveMutex m_chainstate_mutex with Mutex (w0xlt)
ddeefee refactor: add negative TS annotations for `m_chainstate_mutex` (w0xlt)
1dfd31b scripted-diff: rename m_cs_chainstate -> m_chainstate_mutex (w0xlt)

Pull request description:

  This PR is related to bitcoin#19303 and gets rid of the `RecursiveMutex m_cs_chainstate`.

  `m_cs_chainstate` is only held in `ActivateBestChain()` and `InvalidateBlock()`.
  So apparently there is no recursion involved, so the `m_cs_chainstate` can be a non-recursive mutex.

ACKs for top commit:
  hebasto:
    ACK 020acea, I have reviewed the code and it looks OK, I agree it can be merged.
  theStack:
    Code-review ACK 020acea 🌴
  shaavan:
    reACK 020acea

Tree-SHA512: c7c16e727e326df3410514915ce753a2a5e1da78857ef965ef683e36251e1b73c9cced4cd5231b04dbe2be0ea14084f6731b4d7a4d9a8e086e982b985e37e4b4

Author: MarcoFalke

src/validation.cpp

@@ -2847,6 +2847,8 @@ static void LimitValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main) {
 
 bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr<const CBlock> 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
@@ -2856,8 +2858,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;
@@ -2976,6 +2978,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;
@@ -2987,7 +2991,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

src/validation.h

@@ -471,10 +471,11 @@ class CChainState
     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_cs_chainstate;
+    Mutex m_chainstate_mutex;
 
     /**
      * Whether this chainstate is undergoing initial block download.
@@ -638,7 +639,7 @@ class CChainState
      */
     bool ActivateBestChain(
         BlockValidationState& state,
-        std::shared_ptr<const CBlock> pblock = nullptr) LOCKS_EXCLUDED(cs_main);
+        std::shared_ptr<const CBlock> pblock = nullptr) LOCKS_EXCLUDED(m_chainstate_mutex, cs_main);
 
     bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, BlockValidationState& state, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
@@ -658,7 +659,7 @@ class CChainState
      */
     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);