From de17997621cce2784af63dd95b2027884d58fcf1 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Tue, 17 May 2022 08:44:06 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#24062: refactor: replace RecursiveMutex `m_most_recent_block_mutex` with Mutex 83003ffe049a432f6fa4127e054f073127e70b90 refactor: replace RecursiveMutex `m_most_recent_block_mutex` with Mutex (Sebastian Falbesoner) 8edd0d31ac683378135a9839e5d4172b82f8f5b8 refactor: reduce scope of lock `m_most_recent_block_mutex` (Sebastian Falbesoner) Pull request description: This PR is related to #19303 and gets rid of the RecursiveMutex `m_most_recent_block_mutex`. All of the critical sections (5 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex: https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1650-L1655 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1861-L1865 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3149-L3152 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3201-L3206 https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L4763-L4769 The scope of the last critical section is reduced in the first commit, in order to avoid calling the non-trivial method `CConnman::PushMessage` while the lock is held. ACKs for top commit: furszy: Code ACK 83003ffe with a small comment. hebasto: ACK 83003ffe049a432f6fa4127e054f073127e70b90 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/24062/commits/83003ffe049a432f6fa4127e054f073127e70b90 Tree-SHA512: 3df290cafd2f6c4d40afb9f14e822a77d9c1828e66f5e2233f3ac1deccc2b0a8290bc5fb8eb992f49d39e887b50bc0e9aad63e05db2d870791a8d409fb95695f --- src/net_processing.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e0af64376b..f47d6b65f4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -906,7 +906,7 @@ private: // All of the following cache a recent block, and are protected by m_most_recent_block_mutex - RecursiveMutex m_most_recent_block_mutex; + Mutex m_most_recent_block_mutex; std::shared_ptr m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); @@ -5676,15 +5676,16 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LogPrint(BCLog::NET, "%s sending header-and-ids %s to peer=%d\n", __func__, vHeaders.front().GetHash().ToString(), pto->GetId()); - bool fGotBlockFromCache = false; + std::optional cached_cmpctblock_msg; { LOCK(m_most_recent_block_mutex); if (m_most_recent_block_hash == pBestIndex->GetBlockHash()) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block)); - fGotBlockFromCache = true; + cached_cmpctblock_msg = msgMaker.Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block); } } - if (!fGotBlockFromCache) { + if (cached_cmpctblock_msg.has_value()) { + m_connman.PushMessage(pto, std::move(cached_cmpctblock_msg.value())); + } else { CBlock block; bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams); assert(ret);