Skip to content

Commit 7be143a

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#22824: refactor: remove RecursiveMutex cs_nBlockSequenceId
0bd882b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner) Pull request description: The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member: https://github.com/bitcoin/bitcoin/blob/83daf47898f8a79cb20d20316c64becd564cf54c/src/validation.cpp#L2973-L2976 ~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~ ~~This is related to #19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR #3370, commit 75f51f2) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7), probably also because the compilers didn't support them properly earlier.~~ At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main. ACKs for top commit: Zero-1729: ACK 0bd882b promag: Code review ACK 0bd882b. hebasto: ACK 0bd882b Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
2 parents 83daf47 + 0bd882b commit 7be143a

File tree

2 files changed

+3
-7
lines changed

2 files changed

+3
-7
lines changed

src/validation.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,10 +2970,7 @@ void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pi
29702970
CBlockIndex *pindex = queue.front();
29712971
queue.pop_front();
29722972
pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx;
2973-
{
2974-
LOCK(cs_nBlockSequenceId);
2975-
pindex->nSequenceId = nBlockSequenceId++;
2976-
}
2973+
pindex->nSequenceId = nBlockSequenceId++;
29772974
if (m_chain.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) {
29782975
setBlockIndexCandidates.insert(pindex);
29792976
}

src/validation.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,8 @@ class CChainState
558558
* Every received block is assigned a unique and increasing identifier, so we
559559
* know which one to give priority in case of a fork.
560560
*/
561-
RecursiveMutex cs_nBlockSequenceId;
562561
/** Blocks loaded from disk are assigned id 0, so start the counter at 1. */
563-
int32_t nBlockSequenceId = 1;
562+
int32_t nBlockSequenceId GUARDED_BY(::cs_main) = 1;
564563
/** Decreasing counter (used by subsequent preciousblock calls). */
565564
int32_t nBlockReverseSequenceId = -1;
566565
/** chainwork for the last block that preciousblock has been applied to. */
@@ -749,7 +748,7 @@ class CChainState
749748

750749
void PruneBlockIndexCandidates();
751750

752-
void UnloadBlockIndex();
751+
void UnloadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
753752

754753
/** Check whether we are doing an initial block download (synchronizing from disk or network) */
755754
bool IsInitialBlockDownload() const;

0 commit comments

Comments
 (0)