From f92dc6557a153b390a1ae1d0808ff7ed5d02c66e Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Wed, 27 Jan 2021 16:20:59 -0500
Subject: [PATCH] validation: Guard the active_chainstate with cs_main

This avoids a potential race-condition where a thread is reading the
ChainstateManager::m_active_chainstate pointer while another one is
writing to it. There is no portable guarantee that reading/writing the
pointer is thread-safe.

This is also done in way that mimics ::ChainstateActive(), so the
transition from that function to this method is easy.

More discussion:
1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027
2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961
3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522
4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695
---
 src/validation.cpp | 9 +++++++--
 src/validation.h   | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index eeb99f0f0ab..d9bee575db5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5143,6 +5143,7 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin
 }
 
 Optional<uint256> ChainstateManager::SnapshotBlockhash() const {
+    LOCK(::cs_main);  // for m_active_chainstate access
     if (m_active_chainstate != nullptr) {
         // If a snapshot chainstate exists, it will always be our active.
         return m_active_chainstate->m_from_snapshot_blockhash;
@@ -5189,13 +5190,14 @@ CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const
 
 CChainState& ChainstateManager::ActiveChainstate() const
 {
+    LOCK(::cs_main);
     assert(m_active_chainstate);
     return *m_active_chainstate;
 }
 
 bool ChainstateManager::IsSnapshotActive() const
 {
-    return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get();
+    return m_snapshot_chainstate && WITH_LOCK(::cs_main, return m_active_chainstate) == m_snapshot_chainstate.get();
 }
 
 CChainState& ChainstateManager::ValidatedChainstate() const
@@ -5226,7 +5228,10 @@ void ChainstateManager::Reset()
 {
     m_ibd_chainstate.reset();
     m_snapshot_chainstate.reset();
-    m_active_chainstate = nullptr;
+    {
+        LOCK(::cs_main);
+        m_active_chainstate = nullptr;
+    }
     m_snapshot_validated = false;
 }
 
diff --git a/src/validation.h b/src/validation.h
index d3fbabe1a2b..5d060ec753c 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -817,7 +817,7 @@ private:
     //! This is especially important when, e.g., calling ActivateBestChain()
     //! on all chainstates because we are not able to hold ::cs_main going into
     //! that call.
-    CChainState* m_active_chainstate{nullptr};
+    CChainState* m_active_chainstate GUARDED_BY(::cs_main) {nullptr};
 
     //! If true, the assumed-valid chainstate has been fully validated
     //! by the background validation chainstate.