diff options
| author | Carl Dong <contact@carldong.me> | 2021-01-27 16:20:59 -0500 |
|---|---|---|
| committer | Carl Dong <contact@carldong.me> | 2021-01-28 14:14:22 -0500 |
| commit | f92dc6557a153b390a1ae1d0808ff7ed5d02c66e (patch) | |
| tree | d8f634761b2bd2a792847625776aa24fc12fd136 | |
| parent | e130ff38c91ad3527924e6195325f6ae7ce5d77c (diff) | |
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
| -rw-r--r-- | src/validation.cpp | 9 | ||||
| -rw-r--r-- | src/validation.h | 2 |
2 files changed, 8 insertions, 3 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index eeb99f0f0a..d9bee575db 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 d3fbabe1a2..5d060ec753 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. |