diff options
author | Ava Chow <github@achow101.com> | 2024-04-25 14:05:37 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-04-25 14:12:13 -0400 |
commit | 0e2e7d1a355ab6d4c483e192a2f881a5beef2381 (patch) | |
tree | bd4c923059dd0f98347e6a28716bbf328eabcee5 | |
parent | 206629517270f6a51cd38a27552e71405b661c55 (diff) | |
parent | 65951e0418c53cbbf30b9ee85e24ccaf729088a1 (diff) | |
download | bitcoin-0e2e7d1a355ab6d4c483e192a2f881a5beef2381.tar.xz |
Merge bitcoin/bitcoin#29867: index: race fix, lock cs_main while 'm_synced' is subject to change
65951e0418c53cbbf30b9ee85e24ccaf729088a1 index: race fix, lock cs_main while 'm_synced' is subject to change (Ryan Ofsky)
Pull request description:
Fixes #29831 and #29863. Thanks to Marko for the detailed description of the issue.
The race occurs because a block could be connected and its event signaled in-between reading the 'next block' and setting `m_synced` during the index initial synchronization. This is because `cs_main` is not locked through the process of determining the final index sync state.
To address the issue, the `m_synced` flag set has been moved under `cs_main` guard.
ACKs for top commit:
fjahr:
Code review ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1
achow101:
ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1
ryanofsky:
Code review ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1
Tree-SHA512: 77286e22de164a27939d2681b7baa6552eb75e99c541d3b9631f4340d7dd01742667c86899b6987fd2d97799d959e0a913a7749b2b69d9e50505128cd3ae0e69
-rw-r--r-- | src/index/base.cpp | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/src/index/base.cpp b/src/index/base.cpp index a203ce4a9f..e66c89f9e4 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -160,12 +160,24 @@ void BaseIndex::Sync() } const CBlockIndex* pindex_next = WITH_LOCK(cs_main, return NextSyncBlock(pindex, m_chainstate->m_chain)); + // If pindex_next is null, it means pindex is the chain tip, so + // commit data indexed so far. if (!pindex_next) { SetBestBlockIndex(pindex); // No need to handle errors in Commit. See rationale above. Commit(); - m_synced = true; - break; + + // If pindex is still the chain tip after committing, exit the + // sync loop. It is important for cs_main to be locked while + // setting m_synced = true, otherwise a new block could be + // attached while m_synced is still false, and it would not be + // indexed. + LOCK(::cs_main); + pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain); + if (!pindex_next) { + m_synced = true; + break; + } } if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); |