From c75e3d2772b00acc3850f72a8cb733a0345aa773 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Thu, 13 Oct 2022 15:19:57 -0400 Subject: [PATCH 1/6] rest: reduce LOCK(cs_main) scope in rest_block --- src/rest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index a10d8a433f..033e93468e 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -305,8 +305,10 @@ static bool rest_block(const std::any& context, if (chainman.m_blockman.IsBlockPruned(pblockindex)) return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); - if (!ReadBlockFromDisk(block, pblockindex, chainman.GetParams().GetConsensus())) - return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); + } + + if (!ReadBlockFromDisk(block, pblockindex, chainman.GetParams().GetConsensus())) { + return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } switch (rf) { From 7d253c943f44612431be894b198ffb49ff988fff Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Tue, 6 Dec 2022 15:06:03 -0500 Subject: [PATCH 2/6] zmq: remove LOCK(cs_main) from NotifyBlock --- src/zmq/zmqpublishnotifier.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index eaf3455296..c785a929d3 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -248,18 +248,14 @@ bool CZMQPublishRawBlockNotifier::NotifyBlock(const CBlockIndex *pindex) const Consensus::Params& consensusParams = Params().GetConsensus(); CDataStream ss(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); - { - LOCK(cs_main); - CBlock block; - if(!ReadBlockFromDisk(block, pindex, consensusParams)) - { - zmqError("Can't read block from disk"); - return false; - } - - ss << block; + CBlock block; + if (!ReadBlockFromDisk(block, pindex, consensusParams)) { + zmqError("Can't read block from disk"); + return false; } + ss << block; + return SendZmqMessage(MSG_RAWBLOCK, &(*ss.begin()), ss.size()); } From f00808e932c1f67bc76702a3f05778074b10025c Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 17 Oct 2022 09:10:34 -0400 Subject: [PATCH 3/6] rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblock --- src/rpc/blockchain.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9c8d19722b..d95953f7d3 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -579,18 +579,20 @@ static RPCHelpMan getblockheader() }; } -static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex* pblockindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex* pblockindex) { - AssertLockHeld(::cs_main); CBlock block; - if (blockman.IsBlockPruned(pblockindex)) { - throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); + { + LOCK(cs_main); + if (blockman.IsBlockPruned(pblockindex)) { + throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); + } } if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) { // Block not found on disk. This could be because we have the block // header in our index but not yet have the block or did not accept the - // block. + // block. Or if the block was pruned right after we released the lock above. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); } @@ -721,7 +723,6 @@ static RPCHelpMan getblock() } } - CBlock block; const CBlockIndex* pblockindex; const CBlockIndex* tip; ChainstateManager& chainman = EnsureAnyChainman(request.context); @@ -733,10 +734,10 @@ static RPCHelpMan getblock() if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - - block = GetBlockChecked(chainman.m_blockman, pblockindex); } + const CBlock block{GetBlockChecked(chainman.m_blockman, pblockindex)}; + if (verbosity <= 0) { CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION | RPCSerializationFlags()); From efd82aec8a2dd0fca8f2597c3f84cefe057d1243 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 17 Oct 2022 09:11:50 -0400 Subject: [PATCH 4/6] rpc: reduce LOCK(cs_main) scope in blockToJSON --- src/rpc/blockchain.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d95953f7d3..39242ce374 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -181,7 +181,8 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn case TxVerbosity::SHOW_DETAILS: case TxVerbosity::SHOW_DETAILS_AND_PREVOUT: CBlockUndo blockUndo; - const bool have_undo{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex))}; + const bool is_not_pruned{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex))}; + const bool have_undo{is_not_pruned && UndoReadFromDisk(blockUndo, blockindex)}; for (size_t i = 0; i < block.vtx.size(); ++i) { const CTransactionRef& tx = block.vtx.at(i); From 4d92b5aabaf371225cdc37a7b114adc040bf4da5 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 17 Oct 2022 09:12:10 -0400 Subject: [PATCH 5/6] rpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstats --- src/rpc/blockchain.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 39242ce374..784fb64d36 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -600,16 +600,18 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex* pblocki return block; } -static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex* pblockindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex* pblockindex) { - AssertLockHeld(::cs_main); CBlockUndo blockUndo; // The Genesis block does not have undo data if (pblockindex->nHeight == 0) return blockUndo; - if (blockman.IsBlockPruned(pblockindex)) { - throw JSONRPCError(RPC_MISC_ERROR, "Undo data not available (pruned data)"); + { + LOCK(cs_main); + if (blockman.IsBlockPruned(pblockindex)) { + throw JSONRPCError(RPC_MISC_ERROR, "Undo data not available (pruned data)"); + } } if (!UndoReadFromDisk(blockUndo, pblockindex)) { @@ -1799,7 +1801,6 @@ static RPCHelpMan getblockstats() [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { ChainstateManager& chainman = EnsureAnyChainman(request.context); - LOCK(cs_main); const CBlockIndex& pindex{*CHECK_NONFATAL(ParseHashOrHeight(request.params[0], chainman))}; std::set stats; From d7f61e7d5909da7227c9e34be06ce9eb872ba074 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 17 Oct 2022 09:12:20 -0400 Subject: [PATCH 6/6] rpc: reduce LOCK(cs_main) scope in gettxoutproof --- src/rpc/txoutproof.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index cd8b49bfe1..8c5468634d 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -84,13 +84,13 @@ static RPCHelpMan gettxoutproof() g_txindex->BlockUntilSyncedToCurrentChain(); } - LOCK(cs_main); - if (pblockindex == nullptr) { const CTransactionRef tx = GetTransaction(/*block_index=*/nullptr, /*mempool=*/nullptr, *setTxids.begin(), chainman.GetConsensus(), hashBlock); if (!tx || hashBlock.IsNull()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); } + + LOCK(cs_main); pblockindex = chainman.m_blockman.LookupBlockIndex(hashBlock); if (!pblockindex) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt");