highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-21 15:50:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Add lock annotations to Active* methods

Browse Source 
This is a refactor, putting the burden to think about thread safety to
the caller. Otherwise, there is a risk that the caller will assume
thread safety where none exists, as is evident in the previous two
commits.
This commit is contained in:
MacroFake
2022-05-06 16:58:53 +02:00
parent fac15ff673
commit fac04cb6ba
11 changed files with 44 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/test/interfaces_tests.cpp
View File

@@ -17,6 +17,7 @@ BOOST_FIXTURE_TEST_SUITE(interfaces_tests, TestChain100Setup)
BOOST_AUTO_TEST_CASE(findBlock)
{
LOCK(Assert(m_node.chainman)->GetMutex());
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
@@ -61,6 +62,7 @@ BOOST_AUTO_TEST_CASE(findBlock)
BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
{
LOCK(Assert(m_node.chainman)->GetMutex());
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
uint256 hash;
@@ -73,6 +75,7 @@ BOOST_AUTO_TEST_CASE(findFirstBlockWithTimeAndHeight)
BOOST_AUTO_TEST_CASE(findAncestorByHeight)
{
LOCK(Assert(m_node.chainman)->GetMutex());
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
uint256 hash;
@@ -83,6 +86,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHeight)
BOOST_AUTO_TEST_CASE(findAncestorByHash)
{
LOCK(Assert(m_node.chainman)->GetMutex());
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
int height = -1;
@@ -94,7 +98,7 @@ BOOST_AUTO_TEST_CASE(findAncestorByHash)
BOOST_AUTO_TEST_CASE(findCommonAncestor)
{
auto& chain = m_node.chain;
const CChain& active = Assert(m_node.chainman)->ActiveChain();
const CChain& active = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return Assert(m_node.chainman)->ActiveChain());
auto* orig_tip = active.Tip();
for (int i = 0; i < 10; ++i) {
BlockValidationState state;

5
src/test/validation_block_tests.cpp
View File

@@ -234,7 +234,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
// Run the test multiple times
for (int test_runs = 3; test_runs > 0; --test_runs) {
BOOST_CHECK_EQUAL(last_mined->GetHash(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
BOOST_CHECK_EQUAL(last_mined->GetHash(), WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockHash()));
// Later on split from here
const uint256 split_hash{last_mined->hashPrevBlock};
@@ -316,7 +316,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
ProcessBlock(b);
}
// Check that the reorg was eventually successful
BOOST_CHECK_EQUAL(last_mined->GetHash(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
BOOST_CHECK_EQUAL(last_mined->GetHash(), WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockHash()));
// We can join the other thread, which returns when the reorg was successful
rpc_thread.join();
@@ -325,6 +325,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
BOOST_AUTO_TEST_CASE(witness_commitment_index)
{
LOCK(Assert(m_node.chainman)->GetMutex());
CScript pubKey;
pubKey << 1 << OP_TRUE;
auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(pubKey);

16
src/test/validation_chainstatemanager_tests.cpp
View File

@@ -49,12 +49,12 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
auto all = manager.GetAll();
BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end());
auto& active_chain = manager.ActiveChain();
auto& active_chain = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain);
BOOST_CHECK_EQUAL(manager.ActiveHeight(), -1);
BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), -1);
auto active_tip = manager.ActiveTip();
auto active_tip = WITH_LOCK(manager.GetMutex(), return manager.ActiveTip());
auto exp_tip = c1.m_chain.Tip();
BOOST_CHECK_EQUAL(active_tip, exp_tip);
@@ -84,12 +84,12 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
auto all2 = manager.GetAll();
BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end());
auto& active_chain2 = manager.ActiveChain();
auto& active_chain2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
BOOST_CHECK_EQUAL(&active_chain2, &c2.m_chain);
BOOST_CHECK_EQUAL(manager.ActiveHeight(), 0);
BOOST_CHECK_EQUAL(WITH_LOCK(manager.GetMutex(), return manager.ActiveHeight()), 0);
auto active_tip2 = manager.ActiveTip();
auto active_tip2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveTip());
auto exp_tip2 = c2.m_chain.Tip();
BOOST_CHECK_EQUAL(active_tip2, exp_tip2);
@@ -236,7 +236,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
BOOST_CHECK(WITH_LOCK(::cs_main, return !chainman.ActiveChain().Genesis()->IsAssumedValid()));
const AssumeutxoData& au_data = *ExpectedAssumeutxo(snapshot_height, ::Params());
const CBlockIndex* tip = chainman.ActiveTip();
const CBlockIndex* tip = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip());
BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx);
@@ -335,7 +335,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
const int assumed_valid_start_idx = last_assumed_valid_idx - expected_assumed_valid;
CBlockIndex* validated_tip{nullptr};
CBlockIndex* assumed_tip{chainman.ActiveChain().Tip()};
CBlockIndex* assumed_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())};
auto reload_all_block_indexes = [&]() {
for (CChainState* cs : chainman.GetAll()) {