mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-14 23:37:51 +02:00
net: move cs_main up in FetchBlock to fix rpc assert crash
1. FetchBlock runs in a http worker thread. It acquires a PeerRef, locks cs_main, then may later call BlockRequested which asserts that CNodeState exists for the peer. 2. FinalizeNode may run in either the bitcoind or b-net threads. It locks cs_main, fetches a PeerRef from RemovePeer, fetches a CNodeState, and later removes it from m_node_states. Because of the lock placement in FetchBlock, the http worker thread in 1) can acquire a valid PeerRef and block while the b-net thread in 2) is cleaning up the peer in FinalizeNode. When the worker thread later acquires cs_main, it may crash in BlockRequested since no CNodeState exists. Fix this by acquiring the lock earlier in FetchBlock. The lock can be replaced with a net-specific lock when the remaining CNodeState fields are moved to Peer.
This commit is contained in:
@@ -1977,6 +1977,12 @@ util::Expected<void, std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, co
|
||||
{
|
||||
if (m_chainman.m_blockman.LoadingBlocks()) return util::Unexpected{"Loading blocks ..."};
|
||||
|
||||
// The lock must be taken here before fetching Peer so another thread does
|
||||
// not delete the CNodeState from under the current thread, causing an
|
||||
// assertion failure in BlockRequested. This lock can be replaced with a
|
||||
// net-specific lock when more of CNodeState is moved into Peer.
|
||||
LOCK(cs_main);
|
||||
|
||||
// Ensure this peer exists and hasn't been disconnected
|
||||
PeerRef peer = GetPeerRef(peer_id);
|
||||
if (peer == nullptr) return util::Unexpected{"Peer does not exist"};
|
||||
@@ -1984,8 +1990,6 @@ util::Expected<void, std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, co
|
||||
// Ignore pre-segwit peers
|
||||
if (!CanServeWitnesses(*peer)) return util::Unexpected{"Pre-SegWit peer"};
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
// Forget about all prior requests
|
||||
RemoveBlockRequest(block_index.GetBlockHash(), std::nullopt);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user