highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-08-28 05:56:33 +02:00
Code Issues Packages Projects Releases Wiki Activity

locks: Annotate CTxMemPool::check to require cs_main

Browse Source 
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
calls GetSpendHeight which locks cs_main. This can potentially cause an
undesirable lock invesion since CTxMemPool's cs is supposed to be locked
after cs_main.

This does not cause us any problems right now because all callers of
CTxMemPool already lock cs_main before calling CTxMemPool::check, which
means that the LOCK(cs_main) in GetSpendHeight becomes benign.

However, it is currently possible for new code to be added which calls
CTxMemPool::check without locking cs_main (which would be dangerous).
Therefore we should make it explicit that cs_main needs to be held
before calling CTxMemPool::check.

NOTE: After all review-only assertions are removed in "#20158 |
      tree-wide: De-globalize ChainstateManager", and assuming that we
      keep the changes in "validation: Pass in spendheight to
      CTxMemPool::check", we can re-evaluate to see if this annotation
      is still necessary.
This commit is contained in:
Carl Dong
2021-01-20 14:43:27 -05:00
parent 80486e7e2d
commit b396467053
2 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/txmempool.cpp
View File

@@ -618,6 +618,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
if (GetRand(m_check_ratio) >= 1) return; if (GetRand(m_check_ratio) >= 1) return;
AssertLockHeld(::cs_main);
LOCK(cs); LOCK(cs);
LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size()); LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size());

2
src/txmempool.h
View File

@@ -602,7 +602,7 @@ public:
* all inputs are in the mapNextTx array). If sanity-checking is turned off, * all inputs are in the mapNextTx array). If sanity-checking is turned off,
* check does nothing. * check does nothing.
*/ */
void check(const CCoinsViewCache *pcoins) const; void check(const CCoinsViewCache *pcoins) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
// addUnchecked must updated state for all ancestors of a given transaction, // addUnchecked must updated state for all ancestors of a given transaction,
// to track size/count of descendant transactions. First version of // to track size/count of descendant transactions. First version of