[net processing] Improve documentation for Peer destruction/locking

Summary:
As per title, only comments.

Partial backport of [[bitcoin/bitcoin#19829 | core#19829]]:
bitcoin/bitcoin@717a374

Ref T1696.

Test Plan: Read the comments.

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Maniphest Tasks: T1696

Differential Revision: https://reviews.bitcoinabc.org/D10865

Author: Fabcien

src/net_processing.cpp

@@ -1119,6 +1119,11 @@ void PeerManager::FinalizeNode(const Config &config, const CNode &node,
         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,

src/net_processing.h

@@ -56,6 +56,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.
@@ -295,7 +297,10 @@ class PeerManager final : public CValidationInterface,
     //! Next time to check for stale tip
     int64_t m_stale_tip_check_time;
 
-    /** 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