Skip to content

Commit

Permalink
Remove cs_main lock annotation from ChainstateManager.m_blockman
Browse files Browse the repository at this point in the history
Summary:
BlockManager is a large data structure, and cs_main is not required to
take its address or access every part of it. Individual BlockManager
fields and methods which do require cs_main like m_block_index and
LookupBlockIndex are already annotated separately, and these other
annotations describe locking requirements more accurately and do a
better job enforcing thread safety.

Since cs_main is not needed to access the address of the m_block object,
this commit drops cs_main LOCK calls which were added pointlessly to
satisfy this annotation in the past.

Co-authored-by: Carl Dong <[email protected]>

This is a backport of [[bitcoin/bitcoin#24024 | core#24024]]

Test Plan:
With clang and Debug
`ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12909
  • Loading branch information
ryanofsky authored and PiRK committed Dec 19, 2022
1 parent 854f52f commit 593aa43
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 10 deletions.
9 changes: 3 additions & 6 deletions src/test/fuzz/coins_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,9 @@ void test_one_input(const std::vector<uint8_t> &buffer) {
CCoinsStats stats{CoinStatsHashType::HASH_SERIALIZED};
bool expected_code_path = false;
try {
(void)GetUTXOStats(
&coins_view_cache,
WITH_LOCK(::cs_main,
return std::ref(
g_setup->m_node.chainman->m_blockman)),
stats);
(void)GetUTXOStats(&coins_view_cache,
g_setup->m_node.chainman->m_blockman,
stats);
} catch (const std::logic_error &) {
expected_code_path = true;
}
Expand Down
4 changes: 1 addition & 3 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6037,9 +6037,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
CCoinsViewDB *snapshot_coinsdb =
WITH_LOCK(::cs_main, return &snapshot_chainstate.CoinsDB());

if (!GetUTXOStats(snapshot_coinsdb,
WITH_LOCK(::cs_main, return std::ref(m_blockman)), stats,
breakpoint_fnc)) {
if (!GetUTXOStats(snapshot_coinsdb, m_blockman, stats, breakpoint_fnc)) {
LogPrintf("[snapshot] failed to generate coins stats\n");
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ class ChainstateManager {
std::thread m_load_block;
//! A single BlockManager instance is shared across each constructed
//! chainstate to avoid duplicating block metadata.
node::BlockManager m_blockman GUARDED_BY(::cs_main);
node::BlockManager m_blockman;

/**
* In order to efficiently track invalidity of headers, we keep the set of
Expand Down

0 comments on commit 593aa43

Please sign in to comment.