merge bitcoin#24177: add missing thread safety lock assertions

This commit is contained in:
Kittywhiskers Van Gogh 2024-07-04 18:05:22 +00:00
parent 678e67c505
commit 18aa55be1e
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
3 changed files with 68 additions and 19 deletions

View File

@ -269,14 +269,18 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
// instead of unit tests, but for now we need these here.
RegisterAllCoreRPCCommands(tableRPC);
m_node.chainman->InitializeChainstate(m_node.mempool.get(), *m_node.evodb, m_node.chain_helper, llmq::chainLocksHandler, llmq::quorumInstantSendManager);
m_node.chainman->ActiveChainstate().InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
assert(!m_node.chainman->ActiveChainstate().CanFlushToDisk());
m_node.chainman->ActiveChainstate().InitCoinsCache(1 << 23);
assert(m_node.chainman->ActiveChainstate().CanFlushToDisk());
if (!m_node.chainman->ActiveChainstate().LoadGenesisBlock()) {
throw std::runtime_error("LoadGenesisBlock failed.");
{
LOCK(::cs_main);
m_node.chainman->InitializeChainstate(m_node.mempool.get(), *m_node.evodb, m_node.chain_helper, llmq::chainLocksHandler, llmq::quorumInstantSendManager);
m_node.chainman->ActiveChainstate().InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
assert(!m_node.chainman->ActiveChainstate().CanFlushToDisk());
m_node.chainman->ActiveChainstate().InitCoinsCache(1 << 23);
assert(m_node.chainman->ActiveChainstate().CanFlushToDisk());
if (!m_node.chainman->ActiveChainstate().LoadGenesisBlock()) {
throw std::runtime_error("LoadGenesisBlock failed.");
}
}
m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);

View File

@ -324,8 +324,10 @@ static bool ContextualCheckTransaction(const CTransaction& tx, TxValidationState
static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams);
static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache, size_t limit, std::chrono::seconds age)
EXCLUSIVE_LOCKS_REQUIRED(pool.cs, ::cs_main)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs)
{
AssertLockHeld(::cs_main);
AssertLockHeld(pool.cs);
int expired = pool.Expire(GetTime<std::chrono::seconds>() - age);
if (expired != 0) {
LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired);
@ -520,8 +522,10 @@ private:
bool Finalize(const ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
// Compare a package's feerate against minimum allowed.
bool CheckFeeRate(size_t package_size, CAmount package_fee, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs)
bool CheckFeeRate(size_t package_size, CAmount package_fee, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_pool.cs)
{
AssertLockHeld(::cs_main);
AssertLockHeld(m_pool.cs);
CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size);
if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool min fee not met", strprintf("%d < %d", package_fee, mempoolRejectFee));
@ -552,6 +556,8 @@ private:
bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_pool.cs);
const CTransactionRef& ptx = ws.m_ptx;
const CTransaction& tx = *ws.m_ptx;
const uint256& hash = ws.m_hash;
@ -756,6 +762,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_pool.cs);
const CTransaction& tx = *ws.m_ptx;
TxValidationState& state = ws.m_state;
@ -772,6 +780,8 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws, Prec
bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws, PrecomputedTransactionData& txdata)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_pool.cs);
const CTransaction& tx = *ws.m_ptx;
const uint256& hash = ws.m_hash;
TxValidationState& state = ws.m_state;
@ -803,6 +813,8 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws, P
bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_pool.cs);
const CTransaction& tx = *ws.m_ptx;
const uint256& hash = ws.m_hash;
TxValidationState& state = ws.m_state;
@ -941,8 +953,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CChainState& active_chainstate,
const CTransactionRef &tx, int64_t nAcceptTime,
bool bypass_limits, bool test_accept)
EXCLUSIVE_LOCKS_REQUIRED(cs_main)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
std::vector<COutPoint> coins_to_uncache;
MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache, test_accept };
@ -1177,6 +1190,7 @@ CoinsViews::CoinsViews(
void CoinsViews::InitCache()
{
AssertLockHeld(::cs_main);
m_cacheview = std::make_unique<CCoinsViewCache>(&m_catcherview);
}
@ -1214,6 +1228,7 @@ void CChainState::InitCoinsDB(
void CChainState::InitCoinsCache(size_t cache_size_bytes)
{
AssertLockHeld(::cs_main);
assert(m_coins_views != nullptr);
m_coinstip_cache_size_bytes = cache_size_bytes;
m_coins_views->InitCache();
@ -1287,6 +1302,7 @@ void CChainState::CheckForkWarningConditions()
// Called both upon regular invalid block discovery *and* InvalidateBlock
void CChainState::InvalidChainFound(CBlockIndex* pindexNew)
{
AssertLockHeld(cs_main);
statsClient.inc("warnings.InvalidChainFound", 1.0f);
@ -1310,6 +1326,7 @@ void CChainState::InvalidChainFound(CBlockIndex* pindexNew)
void CChainState::ConflictingChainFound(CBlockIndex* pindexNew)
{
AssertLockHeld(cs_main);
statsClient.inc("warnings.ConflictingChainFound", 1.0f);
@ -1328,6 +1345,8 @@ void CChainState::ConflictingChainFound(CBlockIndex* pindexNew)
// which does its own setBlockIndexCandidates manageent.
void CChainState::InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state)
{
AssertLockHeld(cs_main);
statsClient.inc("warnings.InvalidBlockFound", 1.0f);
if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
pindex->nStatus |= BLOCK_FAILED_VALID;
@ -2275,6 +2294,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
CoinsCacheSizeState CChainState::GetCoinsCacheSizeState()
{
AssertLockHeld(::cs_main);
return this->GetCoinsCacheSizeState(
m_coinstip_cache_size_bytes,
gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
@ -2284,6 +2304,7 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(
size_t max_coins_cache_size_bytes,
size_t max_mempool_size_bytes)
{
AssertLockHeld(::cs_main);
const int64_t nMempoolUsage = m_mempool ? m_mempool->DynamicMemoryUsage() : 0;
int64_t cacheSize = CoinsTip().DynamicMemoryUsage();
int64_t nTotalSpace =
@ -2498,6 +2519,7 @@ static void UpdateTipLog(
void CChainState::UpdateTip(const CBlockIndex* pindexNew)
{
AssertLockHeld(::cs_main);
const auto& coins_tip = this->CoinsTip();
// The remainder of the function isn't relevant if we are not acting on
@ -2731,7 +2753,9 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew
* Return the tip of the chain with the most work in it, that isn't
* known to be invalid (it's however far from certain to be valid).
*/
CBlockIndex* CChainState::FindMostWorkChain() {
CBlockIndex* CChainState::FindMostWorkChain()
{
AssertLockHeld(::cs_main);
do {
CBlockIndex *pindexNew = nullptr;
@ -2940,7 +2964,7 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr
// far from a guarantee. Things in the P2P/RPC will often end up calling
// us in the middle of ProcessNewBlock - do not assume pblock is set
// sanely for performance or correctness!
AssertLockNotHeld(cs_main);
AssertLockNotHeld(::cs_main);
// ABC maintains a fair degree of expensive-to-calculate internal state
// because this function periodically releases cs_main so that it does not lock up other threads for too long
@ -3043,6 +3067,8 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr
bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex)
{
AssertLockNotHeld(m_chainstate_mutex);
AssertLockNotHeld(::cs_main);
{
LOCK(cs_main);
if (pindex->nChainWork < m_chain.Tip()->nChainWork) {
@ -3073,6 +3099,7 @@ bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex
bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex)
{
AssertLockNotHeld(m_chainstate_mutex);
AssertLockNotHeld(::cs_main);
// Genesis block can't be invalidated
assert(pindex);
@ -3389,6 +3416,7 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) {
/** Mark a block as having its data received and checked (up to BLOCK_VALID_TRANSACTIONS). */
void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos)
{
AssertLockHeld(cs_main);
pindexNew->nTx = block.vtx.size();
pindexNew->nChainTx = 0;
pindexNew->nFile = pos.nFile;
@ -3529,8 +3557,9 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu
* in ConnectBlock().
* Note that -reindex-chainstate skips the validation that happens here!
*/
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
assert(pindexPrev != nullptr);
const int nHeight = pindexPrev->nHeight + 1;
@ -4153,6 +4182,8 @@ bool CVerifyDB::VerifyDB(
/** Apply the effects of a block on the utxo cache, ignoring that it may already have been applied. */
bool CChainState::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs)
{
AssertLockHeld(cs_main);
assert(m_chain_helper);
// TODO: merge with ConnectBlock
@ -4332,7 +4363,9 @@ bool CChainState::ReplayBlocks()
return true;
}
void CChainState::UnloadBlockIndex() {
void CChainState::UnloadBlockIndex()
{
AssertLockHeld(::cs_main);
nBlockSequenceId = 1;
setBlockIndexCandidates.clear();
}
@ -4500,6 +4533,7 @@ bool CChainState::LoadGenesisBlock()
void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp)
{
AssertLockNotHeld(m_chainstate_mutex);
// Map of disk positions for blocks with unknown parent (only used for reindex)
static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent;
int64_t nStart = GetTimeMillis();
@ -4848,6 +4882,7 @@ void CChainState::CheckBlockIndex()
std::string CChainState::ToString()
{
AssertLockHeld(::cs_main);
CBlockIndex* tip = m_chain.Tip();
return strprintf("Chainstate [%s] @ height %d (%s)",
m_from_snapshot_blockhash ? "snapshot" : "ibd",
@ -4856,6 +4891,7 @@ std::string CChainState::ToString()
bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size)
{
AssertLockHeld(::cs_main);
if (coinstip_size == m_coinstip_cache_size_bytes &&
coinsdb_size == m_coinsdb_cache_size_bytes) {
// Cache sizes are unchanged, no need to continue.
@ -5085,6 +5121,7 @@ CChainState& ChainstateManager::InitializeChainstate(CTxMemPool* mempool,
const std::unique_ptr<llmq::CInstantSendManager>& isman,
const std::optional<uint256>& snapshot_blockhash)
{
AssertLockHeld(::cs_main);
bool is_snapshot = snapshot_blockhash.has_value();
std::unique_ptr<CChainState>& to_modify =
is_snapshot ? m_snapshot_chainstate : m_ibd_chainstate;
@ -5419,6 +5456,7 @@ bool ChainstateManager::IsSnapshotActive() const
void ChainstateManager::Unload()
{
AssertLockHeld(::cs_main);
for (CChainState* chainstate : this->GetAll()) {
chainstate->m_chain.SetTip(nullptr);
chainstate->UnloadBlockIndex();
@ -5441,6 +5479,7 @@ void ChainstateManager::Reset()
void ChainstateManager::MaybeRebalanceCaches()
{
AssertLockHeld(::cs_main);
if (m_ibd_chainstate && !m_snapshot_chainstate) {
LogPrintf("[snapshot] allocating all cache to the IBD chainstate\n");
// Allocate everything to the IBD chainstate.

View File

@ -534,7 +534,9 @@ public:
//! @returns whether or not the CoinsViews object has been fully initialized and we can
//! safely flush this object to disk.
bool CanFlushToDisk() const EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
bool CanFlushToDisk() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
return m_coins_views && m_coins_views->m_cacheview;
}
@ -568,22 +570,25 @@ public:
}
//! @returns A reference to the in-memory cache of the UTXO set.
CCoinsViewCache& CoinsTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
CCoinsViewCache& CoinsTip() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
assert(m_coins_views->m_cacheview);
return *m_coins_views->m_cacheview.get();
}
//! @returns A reference to the on-disk UTXO set database.
CCoinsViewDB& CoinsDB() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
CCoinsViewDB& CoinsDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
return m_coins_views->m_dbview;
}
//! @returns A reference to a wrapped view of the in-memory UTXO set that
//! handles disk read errors gracefully.
CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
return m_coins_views->m_catcherview;
}
@ -939,6 +944,7 @@ public:
BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
return m_blockman.m_block_index;
}