Skip to content

Commit a51a755

Browse files
skeeesfurszy
authored andcommitted
Fix concurrency-related bugs in ActivateBestChain
If multiple threads are invoking ActivateBestChain, it was possible to have them working towards different tips, and we could arrive at a less work tip than we should.  Fix this by introducing a ChainState lock which must be held for the entire duration of ActivateBestChain to enforce exclusion in ABC.
1 parent 198f435 commit a51a755

File tree

1 file changed

+15
-3
lines changed

1 file changed

+15
-3
lines changed

src/validation.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,17 @@ struct CBlockIndexWorkComparator {
147147
CBlockIndex* pindexBestInvalid;
148148

149149
/**
150-
* The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and
151-
* as good as our current tip or better. Entries may be failed, though.
152-
*/
150+
* The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and
151+
* as good as our current tip or better. Entries may be failed, though.
152+
*/
153153
std::set<CBlockIndex*, CBlockIndexWorkComparator> setBlockIndexCandidates;
154154

155+
/**
156+
* the ChainState Mutex
157+
* A lock that must be held when modifying this ChainState - held in ActivateBestChain()
158+
*/
159+
Mutex m_cs_chainstate;
160+
155161
/** All pairs A->B, where A (or one if its ancestors) misses transactions, but B has transactions. */
156162
std::multimap<CBlockIndex*, CBlockIndex*> mapBlocksUnlinked;
157163

@@ -2226,6 +2232,12 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr<const CBlock> pb
22262232
// sanely for performance or correctness!
22272233
AssertLockNotHeld(cs_main);
22282234

2235+
// ABC maintains a fair degree of expensive-to-calculate internal state
2236+
// because this function periodically releases cs_main so that it does not lock up other threads for too long
2237+
// during large connects - and to allow for e.g. the callback queue to drain
2238+
// we use m_cs_chainstate to enforce mutual exclusion so that only one caller may execute this function at a time
2239+
LOCK(m_cs_chainstate);
2240+
22292241
CBlockIndex* pindexNewTip = nullptr;
22302242
CBlockIndex* pindexMostWork = nullptr;
22312243
do {

0 commit comments

Comments
 (0)