aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2020-07-30 17:30:44 +0200
committerMarcoFalke <falke.marco@gmail.com>2020-07-30 17:30:52 +0200
commitad2952d17a2af419a04256b10b53c7377f826a27 (patch)
tree7aefc283a3f710c626ef0eddab02989ce947ac35 /src
parent62d137ac3b701aae36c1aa3aa93a83fd6357fde6 (diff)
parentfae8c28dae747f9c4c6481049742346d18202fc8 (diff)
Merge #19604: Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState
fae8c28dae747f9c4c6481049742346d18202fc8 Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke) fac674db200e6b2d5b32069335fb24e713d7b69f Pass mempool pointer to UnloadBlockIndex (MarcoFalke) faec851b6eb7e65e28cdcae50d6dc86fafa0f91c test: Simplify cs_main locks (MarcoFalke) Pull request description: Split out from #19556 Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556. ACKs for top commit: jnewbery: code review ACK fae8c28dae747f9c4c6481049742346d18202fc8 fjahr: Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8 darosior: Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8 jamesob: ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to)) Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728
Diffstat (limited to 'src')
-rw-r--r--src/init.cpp2
-rw-r--r--src/qt/test/apptests.cpp2
-rw-r--r--src/test/util/setup_common.cpp2
-rw-r--r--src/test/validation_chainstate_tests.cpp4
-rw-r--r--src/test/validation_chainstatemanager_tests.cpp13
-rw-r--r--src/test/validation_flush_tests.cpp22
-rw-r--r--src/validation.cpp12
-rw-r--r--src/validation.h6
8 files changed, 27 insertions, 36 deletions
diff --git a/src/init.cpp b/src/init.cpp
index ad500702d7..6cca21f375 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1567,7 +1567,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
chainman.m_total_coinstip_cache = nCoinCacheUsage;
chainman.m_total_coinsdb_cache = nCoinDBCache;
- UnloadBlockIndex();
+ UnloadBlockIndex(node.mempool);
// new CBlockTreeDB tries to delete the existing file, which
// fails if it's still open from the previous loop. Close it first:
diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp
index 443e2d047d..b880a99baf 100644
--- a/src/qt/test/apptests.cpp
+++ b/src/qt/test/apptests.cpp
@@ -83,7 +83,7 @@ void AppTests::appTests()
// Reset global state to avoid interfering with later tests.
LogInstance().DisconnectTestLogger();
AbortShutdown();
- UnloadBlockIndex();
+ UnloadBlockIndex(/* mempool */ nullptr);
WITH_LOCK(::cs_main, g_chainman.Reset());
}
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 0cd991f453..14f65dcb7c 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -182,9 +182,9 @@ TestingSetup::~TestingSetup()
m_node.connman.reset();
m_node.banman.reset();
m_node.args = nullptr;
+ UnloadBlockIndex(m_node.mempool);
m_node.mempool = nullptr;
m_node.scheduler.reset();
- UnloadBlockIndex();
m_node.chainman->Reset();
m_node.chainman = nullptr;
pblocktree.reset();
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index f531b393b8..2076a1096a 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -34,9 +34,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
return outp;
};
- ENTER_CRITICAL_SECTION(cs_main);
- CChainState& c1 = manager.InitializeChainstate();
- LEAVE_CRITICAL_SECTION(cs_main);
+ CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate());
c1.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index f99191b08f..887a48124f 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -54,10 +54,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
// Create a snapshot-based chainstate.
//
- ENTER_CRITICAL_SECTION(cs_main);
- CChainState& c2 = *WITH_LOCK(::cs_main,
- return &manager.InitializeChainstate(GetRandHash()));
- LEAVE_CRITICAL_SECTION(cs_main);
+ CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(GetRandHash()));
chainstates.push_back(&c2);
c2.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -115,9 +112,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
// Create a legacy (IBD) chainstate.
//
- ENTER_CRITICAL_SECTION(cs_main);
- CChainState& c1 = manager.InitializeChainstate();
- LEAVE_CRITICAL_SECTION(cs_main);
+ CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate());
chainstates.push_back(&c1);
c1.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -134,9 +129,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
// Create a snapshot-based chainstate.
//
- ENTER_CRITICAL_SECTION(cs_main);
- CChainState& c2 = manager.InitializeChainstate(GetRandHash());
- LEAVE_CRITICAL_SECTION(cs_main);
+ CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(GetRandHash()));
chainstates.push_back(&c2);
c2.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
index 7111fc3380..8bac914f05 100644
--- a/src/test/validation_flush_tests.cpp
+++ b/src/test/validation_flush_tests.cpp
@@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
// Without any coins in the cache, we shouldn't need to flush.
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
CoinsCacheSizeState::OK);
// If the initial memory allocations of cacheCoins don't match these common
@@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
}
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
CoinsCacheSizeState::CRITICAL);
BOOST_TEST_MESSAGE("Exiting cache flush tests early due to unsupported arch");
@@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
print_view_mem_usage(view);
BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE);
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
CoinsCacheSizeState::OK);
}
@@ -100,26 +100,26 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
for (int i{0}; i < 4; ++i) {
add_coin(view);
print_view_mem_usage(view);
- if (chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) ==
+ if (chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) ==
CoinsCacheSizeState::CRITICAL) {
break;
}
}
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
CoinsCacheSizeState::CRITICAL);
// Passing non-zero max mempool usage should allow us more headroom.
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
CoinsCacheSizeState::OK);
for (int i{0}; i < 3; ++i) {
add_coin(view);
print_view_mem_usage(view);
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
CoinsCacheSizeState::OK);
}
@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
BOOST_CHECK(usage_percentage >= 0.9);
BOOST_CHECK(usage_percentage < 1);
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10),
CoinsCacheSizeState::LARGE);
}
@@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
for (int i{0}; i < 1000; ++i) {
add_coin(view);
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool),
+ chainstate.GetCoinsCacheSizeState(&tx_pool),
CoinsCacheSizeState::OK);
}
@@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
// preallocated memory that doesn't get reclaimed even after flush.
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0),
CoinsCacheSizeState::CRITICAL);
view.SetBestBlock(InsecureRand256());
@@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
print_view_mem_usage(view);
BOOST_CHECK_EQUAL(
- chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+ chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0),
CoinsCacheSizeState::CRITICAL);
}
diff --git a/src/validation.cpp b/src/validation.cpp
index 545aa0f2d3..3cebf1090e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2227,7 +2227,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
return true;
}
-CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_pool)
+CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool* tx_pool)
{
return this->GetCoinsCacheSizeState(
tx_pool,
@@ -2236,11 +2236,11 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_poo
}
CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(
- const CTxMemPool& tx_pool,
+ const CTxMemPool* tx_pool,
size_t max_coins_cache_size_bytes,
size_t max_mempool_size_bytes)
{
- int64_t nMempoolUsage = tx_pool.DynamicMemoryUsage();
+ const int64_t nMempoolUsage = tx_pool ? tx_pool->DynamicMemoryUsage() : 0;
int64_t cacheSize = CoinsTip().DynamicMemoryUsage();
int64_t nTotalSpace =
max_coins_cache_size_bytes + std::max<int64_t>(max_mempool_size_bytes - nMempoolUsage, 0);
@@ -2279,7 +2279,7 @@ bool CChainState::FlushStateToDisk(
{
bool fFlushForPrune = false;
bool fDoFullFlush = false;
- CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(::mempool);
+ CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&::mempool);
LOCK(cs_LastBlockFile);
if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
if (nManualPruneHeight > 0) {
@@ -4587,13 +4587,13 @@ void CChainState::UnloadBlockIndex() {
// May NOT be used after any connections are up as much
// of the peer-processing logic assumes a consistent
// block index state
-void UnloadBlockIndex()
+void UnloadBlockIndex(CTxMemPool* mempool)
{
LOCK(cs_main);
g_chainman.Unload();
pindexBestInvalid = nullptr;
pindexBestHeader = nullptr;
- mempool.clear();
+ if (mempool) mempool->clear();
vinfoBlockFile.clear();
nLastBlockFile = 0;
setDirtyBlockIndex.clear();
diff --git a/src/validation.h b/src/validation.h
index 53503768aa..534162d64a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -160,7 +160,7 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi
/** Ensures we have a genesis block in the block tree, possibly writing one to disk. */
bool LoadGenesisBlock(const CChainParams& chainparams);
/** Unload database information */
-void UnloadBlockIndex();
+void UnloadBlockIndex(CTxMemPool* mempool);
/** Run an instance of the script checking thread */
void ThreadScriptCheck(int worker_num);
/**
@@ -674,11 +674,11 @@ public:
//! Dictates whether we need to flush the cache to disk or not.
//!
//! @return the state of the size of the coins cache.
- CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool& tx_pool)
+ CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool* tx_pool)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
CoinsCacheSizeState GetCoinsCacheSizeState(
- const CTxMemPool& tx_pool,
+ const CTxMemPool* tx_pool,
size_t max_coins_cache_size_bytes,
size_t max_mempool_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);