Skip to content

Commit 1fbd3d9

Browse files
laanwjknst
authored andcommitted
Merge bitcoin#19854: Avoid locking CTxMemPool::cs recursively in simple cases
020f051 refactor: CTxMemPool::IsUnbroadcastTx() requires CTxMemPool::cs lock (Hennadii Stepanov) 7c4bd03 refactor: CTxMemPool::GetTotalTxSize() requires CTxMemPool::cs lock (Hennadii Stepanov) fa5fcb0 refactor: CTxMemPool::ClearPrioritisation() requires CTxMemPool::cs lock (Hennadii Stepanov) 7140b31 refactor: CTxMemPool::ApplyDelta() requires CTxMemPool::cs lock (Hennadii Stepanov) 66e47e5 refactor: CTxMemPool::UpdateChild() requires CTxMemPool::cs lock (Hennadii Stepanov) 9398077 refactor: CTxMemPool::UpdateParent() requires CTxMemPool::cs lock (Hennadii Stepanov) Pull request description: This is another step to transit `CTxMemPool::cs` from `RecursiveMutex` to `Mutex`. Split out from bitcoin#19306. Only trivial thread safety annotations and lock assertions added. No new locks. No behavior change. Refactoring `const uint256` to `const uint256&` was [requested](bitcoin#19647 (comment)) by **promag**. Please note that now, since bitcoin#19668 has been merged, it is safe to apply `AssertLockHeld()` macros as they do not swallow compile time Thread Safety Analysis warnings. ACKs for top commit: promag: Core review ACK 020f051. jnewbery: Code review ACK 020f051 vasild: ACK 020f051 Tree-SHA512: a31e389142d5a19b25fef0aaf1072a337278564528b5cc9209df88ae548a31440e1b8dd9bae0169fd7aa59ea06e22fe5e0413955386512b83ef1f3e7d941e890
1 parent 852a7a4 commit 1fbd3d9

File tree

2 files changed

+12
-10
lines changed

2 files changed

+12
-10
lines changed

src/txmempool.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,19 +1362,19 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
13621362
LogPrint(BCLog::MEMPOOL, "PrioritiseTransaction: %s feerate += %s\n", hash.ToString(), FormatMoney(nFeeDelta));
13631363
}
13641364

1365-
void CTxMemPool::ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const
1365+
void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const
13661366
{
1367-
LOCK(cs);
1367+
AssertLockHeld(cs);
13681368
std::map<uint256, CAmount>::const_iterator pos = mapDeltas.find(hash);
13691369
if (pos == mapDeltas.end())
13701370
return;
13711371
const CAmount &delta = pos->second;
13721372
nFeeDelta += delta;
13731373
}
13741374

1375-
void CTxMemPool::ClearPrioritisation(const uint256 hash)
1375+
void CTxMemPool::ClearPrioritisation(const uint256& hash)
13761376
{
1377-
LOCK(cs);
1377+
AssertLockHeld(cs);
13781378
mapDeltas.erase(hash);
13791379
}
13801380

@@ -1483,6 +1483,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, bool validFeeEstimat
14831483

14841484
void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add)
14851485
{
1486+
AssertLockHeld(cs);
14861487
setEntries s;
14871488
if (add && mapLinks[entry].children.insert(child).second) {
14881489
cachedInnerUsage += memusage::IncrementalDynamicUsage(s);
@@ -1493,6 +1494,7 @@ void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add)
14931494

14941495
void CTxMemPool::UpdateParent(txiter entry, txiter parent, bool add)
14951496
{
1497+
AssertLockHeld(cs);
14961498
setEntries s;
14971499
if (add && mapLinks[entry].parents.insert(parent).second) {
14981500
cachedInnerUsage += memusage::IncrementalDynamicUsage(s);

src/txmempool.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,8 @@ class CTxMemPool
568568
std::map<uint256, uint256> mapProTxBlsPubKeyHashes;
569569
std::map<COutPoint, uint256> mapProTxCollaterals;
570570

571-
void UpdateParent(txiter entry, txiter parent, bool add);
572-
void UpdateChild(txiter entry, txiter child, bool add);
571+
void UpdateParent(txiter entry, txiter parent, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs);
572+
void UpdateChild(txiter entry, txiter child, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs);
573573

574574
std::vector<indexed_transaction_set::const_iterator> GetSortedDepthAndScore() const EXCLUSIVE_LOCKS_REQUIRED(cs);
575575

@@ -638,8 +638,8 @@ class CTxMemPool
638638

639639
/** Affect CreateNewBlock prioritisation of transactions */
640640
void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta);
641-
void ApplyDelta(const uint256 hash, CAmount &nFeeDelta) const;
642-
void ClearPrioritisation(const uint256 hash);
641+
void ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const EXCLUSIVE_LOCKS_REQUIRED(cs);
642+
void ClearPrioritisation(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs);
643643

644644
/** Get the transaction in the pool that spends the same prevout */
645645
const CTransaction* GetConflictTx(const COutPoint& prevout) const EXCLUSIVE_LOCKS_REQUIRED(cs);
@@ -722,9 +722,9 @@ class CTxMemPool
722722
return mapTx.size();
723723
}
724724

725-
uint64_t GetTotalTxSize() const
725+
uint64_t GetTotalTxSize() const EXCLUSIVE_LOCKS_REQUIRED(cs)
726726
{
727-
LOCK(cs);
727+
AssertLockHeld(cs);
728728
return totalTxSize;
729729
}
730730

0 commit comments

Comments
 (0)