aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/bench/mempool_eviction.cpp4
-rw-r--r--src/sync.h3
-rw-r--r--src/test/blockencodings_tests.cpp6
-rw-r--r--src/test/mempool_tests.cpp10
-rw-r--r--src/test/miner_tests.cpp2
-rw-r--r--src/test/policyestimator_tests.cpp2
-rw-r--r--src/txmempool.h46
-rw-r--r--src/validation.cpp16
8 files changed, 67 insertions, 22 deletions
diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp
index 49ea6e88b5..ac8a182358 100644
--- a/src/bench/mempool_eviction.cpp
+++ b/src/bench/mempool_eviction.cpp
@@ -9,7 +9,7 @@
#include <list>
#include <vector>
-static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs)
+static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs)
{
int64_t nTime = 0;
unsigned int nHeight = 1;
@@ -108,7 +108,7 @@ static void MempoolEviction(benchmark::State& state)
tx7.vout[1].nValue = 10 * COIN;
CTxMemPool pool;
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
// Create transaction references outside the "hot loop"
const CTransactionRef tx1_r{MakeTransactionRef(tx1)};
const CTransactionRef tx2_r{MakeTransactionRef(tx2)};
diff --git a/src/sync.h b/src/sync.h
index 40709bdd7f..3857eda56b 100644
--- a/src/sync.h
+++ b/src/sync.h
@@ -20,7 +20,7 @@
////////////////////////////////////////////////
/*
-CCriticalSection mutex;
+RecursiveMutex mutex;
std::recursive_mutex mutex;
LOCK(mutex);
@@ -104,6 +104,7 @@ public:
* Wrapped mutex: supports recursive locking, but no waiting
* TODO: We should move away from using the recursive lock by default.
*/
+using RecursiveMutex = AnnotatedMixin<std::recursive_mutex>;
typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection;
/** Wrapped mutex: supports waiting but not recursive locking */
diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 309b8d2d06..607af8a32a 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
pool.addUnchecked(entry.FromTx(block.vtx[2]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
@@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
pool.addUnchecked(entry.FromTx(block.vtx[2]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
@@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
TestMemPoolEntryHelper entry;
CBlock block(BuildBlockTestCase());
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
pool.addUnchecked(entry.FromTx(block.vtx[1]));
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 2396aba0f1..23ca9d89ae 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
CTxMemPool testPool;
- LOCK(testPool.cs);
+ LOCK2(cs_main, testPool.cs);
// Nothing in pool, remove should do nothing:
unsigned int poolSize = testPool.size();
@@ -120,7 +120,7 @@ static void CheckSort(CTxMemPool &pool, std::vector<std::string> &sortedOrder) E
BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
{
CTxMemPool pool;
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
/* 3rd highest fee */
@@ -293,7 +293,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
{
CTxMemPool pool;
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
/* 3rd highest fee */
@@ -422,7 +422,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
{
CTxMemPool pool;
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
CMutableTransaction tx1 = CMutableTransaction();
@@ -595,7 +595,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
size_t ancestors, descendants;
CTxMemPool pool;
- LOCK(pool.cs);
+ LOCK2(cs_main, pool.cs);
TestMemPoolEntryHelper entry;
/* Base transaction */
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index f3648e2eee..5ba1df2ec2 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -99,7 +99,7 @@ static bool TestSequenceLocks(const CTransaction &tx, int flags) EXCLUSIVE_LOCKS
// Test suite for ancestor feerate transaction selection.
// Implemented as an additional function, rather than a separate test case,
// to allow reusing the blockchain created in CreateNewBlock_validity.
-static void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::mempool.cs)
+static void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs)
{
// Test the ancestor feerate transaction selection.
TestMemPoolEntryHelper entry;
diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp
index 431b16cfc2..7b274a1658 100644
--- a/src/test/policyestimator_tests.cpp
+++ b/src/test/policyestimator_tests.cpp
@@ -18,7 +18,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
{
CBlockPolicyEstimator feeEst;
CTxMemPool mpool(&feeEst);
- LOCK(mpool.cs);
+ LOCK2(cs_main, mpool.cs);
TestMemPoolEntryHelper entry;
CAmount basefee(2000);
CAmount deltaFee(100);
diff --git a/src/txmempool.h b/src/txmempool.h
index fadb554723..b10c9f099f 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -485,7 +485,43 @@ public:
>
> indexed_transaction_set;
- mutable CCriticalSection cs;
+ /**
+ * This mutex needs to be locked when accessing `mapTx` or other members
+ * that are guarded by it.
+ *
+ * @par Consistency guarantees
+ *
+ * By design, it is guaranteed that:
+ *
+ * 1. Locking both `cs_main` and `mempool.cs` will give a view of mempool
+ * that is consistent with current chain tip (`chainActive` and
+ * `pcoinsTip`) and is fully populated. Fully populated means that if the
+ * current active chain is missing transactions that were present in a
+ * previously active chain, all the missing transactions will have been
+ * re-added to the mempool and should be present if they meet size and
+ * consistency constraints.
+ *
+ * 2. Locking `mempool.cs` without `cs_main` will give a view of a mempool
+ * consistent with some chain that was active since `cs_main` was last
+ * locked, and that is fully populated as described above. It is ok for
+ * code that only needs to query or remove transactions from the mempool
+ * to lock just `mempool.cs` without `cs_main`.
+ *
+ * To provide these guarantees, it is necessary to lock both `cs_main` and
+ * `mempool.cs` whenever adding transactions to the mempool and whenever
+ * changing the chain tip. It's necessary to keep both mutexes locked until
+ * the mempool is consistent with the new chain tip and fully populated.
+ *
+ * @par Consistency bug
+ *
+ * The second guarantee above is not currently enforced, but
+ * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code
+ * in bitcoin currently depends on second guarantee, but it is important to
+ * fix for third party code that needs be able to frequently poll the
+ * mempool without locking `cs_main` and without encountering missing
+ * transactions during reorgs.
+ */
+ mutable RecursiveMutex cs;
indexed_transaction_set mapTx GUARDED_BY(cs);
using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;
@@ -541,8 +577,8 @@ public:
// Note that addUnchecked is ONLY called from ATMP outside of tests
// and any other callers may break wallet's in-mempool tracking (due to
// lack of CValidationInterface::TransactionAddedToMempool callbacks).
- void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs);
- void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs);
+ void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+ void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -594,7 +630,7 @@ public:
* for). Note: vHashesToUpdate should be the set of transactions from the
* disconnected block that have been accepted back into the mempool.
*/
- void UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate);
+ void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/** Try to calculate all in-mempool ancestors of entry.
* (these are all calculated including the tx itself)
@@ -636,7 +672,7 @@ public:
*/
void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) const;
- unsigned long size()
+ unsigned long size() const
{
LOCK(cs);
return mapTx.size();
diff --git a/src/validation.cpp b/src/validation.cpp
index a18e449af6..6a26bf9baa 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -173,7 +173,7 @@ public:
CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
// Block disconnection on our pcoinsTip:
- bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool);
+ bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
// Manual block validity manipulation:
bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main);
@@ -210,9 +210,17 @@ private:
bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs, const CChainParams& params) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
} g_chainstate;
-
-
-CCriticalSection cs_main;
+/**
+ * Mutex to guard access to validation specific variables, such as reading
+ * or changing the chainstate.
+ *
+ * This may also need to be locked when updating the transaction pool, e.g. on
+ * AcceptToMemoryPool. See CTxMemPool::cs comment for details.
+ *
+ * The transaction pool has a separate lock to allow reading from it and the
+ * chainstate at the same time.
+ */
+RecursiveMutex cs_main;
BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex;
CChain& chainActive = g_chainstate.chainActive;
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 19:42:54 +0000