diff options
author | John Newbery <john@johnnewbery.com> | 2020-08-28 21:04:44 +0100 |
---|---|---|
committer | John Newbery <john@johnnewbery.com> | 2020-12-19 14:49:03 +0000 |
commit | 717a374e74b64b7b90bc1b2995e8900212bd0bfe (patch) | |
tree | 98a0e5025ea11a829f10b17475a789aaaba76be6 | |
parent | f1dbf92ff0475a01d20170ea422c1d086acbbc57 (diff) | |
download | bitcoin-717a374e74b64b7b90bc1b2995e8900212bd0bfe.tar.xz |
[net processing] Improve documentation for Peer destruction/locking
Suggested here:
- https://github.com/bitcoin/bitcoin/pull/19607#discussion_r467071878
- https://github.com/bitcoin/bitcoin/pull/19829#discussion_r546116786
-rw-r--r-- | src/net_processing.cpp | 5 | ||||
-rw-r--r-- | src/net_processing.h | 5 |
2 files changed, 9 insertions, 1 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 06fffe5148..41f3dce344 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -791,6 +791,11 @@ void PeerManager::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) { LOCK(cs_main); int misbehavior{0}; { + // We remove the PeerRef from g_peer_map here, but we don't always + // destruct the Peer. Sometimes another thread is still holding a + // PeerRef, so the refcount is >= 1. Be careful not to do any + // processing here that assumes Peer won't be changed before it's + // destructed. PeerRef peer = RemovePeer(nodeid); assert(peer != nullptr); misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); diff --git a/src/net_processing.h b/src/net_processing.h index 12a4e9c38f..3600fe7d63 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -46,6 +46,8 @@ struct CNodeStateStats { * Memory is owned by shared pointers and this object is destructed when * the refcount drops to zero. * + * Mutexes inside this struct must not be held when locking m_peer_mutex. + * * TODO: move most members from CNodeState to this structure. * TODO: move remaining application-layer data members from CNode to this structure. */ @@ -210,7 +212,8 @@ private: * on extra block-relay-only peers. */ bool m_initial_sync_finished{false}; - /** Protects m_peer_map */ + /** Protects m_peer_map. This mutex must not be locked while holding a lock + * on any of the mutexes inside a Peer object. */ mutable Mutex m_peer_mutex; /** * Map of all Peer objects, keyed by peer id. This map is protected |