diff options
| author | Carl Dong <contact@carldong.me> | 2021-01-20 14:43:27 -0500 |
|---|---|---|
| committer | Carl Dong <contact@carldong.me> | 2021-01-20 16:15:03 -0500 |
| commit | b3964670537d0943b8fb2d8f2ea419cbefd4835a (patch) | |
| tree | 878d40ef04249df48be7e6fcd2af8b947f3065fe | |
| parent | 80486e7e2d8c360839cffc5a0c597b5c745433a7 (diff) | |
locks: Annotate CTxMemPool::check to require cs_main
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.
| -rw-r--r-- | src/txmempool.cpp | 1 | ||||
| -rw-r--r-- | src/txmempool.h | 2 |
2 files changed, 2 insertions, 1 deletions
diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 9ae7b921b3..470e665844 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -618,6 +618,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const if (GetRand(m_check_ratio) >= 1) return; + AssertLockHeld(::cs_main); LOCK(cs); LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size()); diff --git a/src/txmempool.h b/src/txmempool.h index 9a1aa9bc2b..0a9cd81ff5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -602,7 +602,7 @@ public: * all inputs are in the mapNextTx array). If sanity-checking is turned off, * 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, // to track size/count of descendant transactions. First version of |