doc: better document NetEventsInterface and the deletion of “CNode"s #32278

1. 
   vasild commented at 4:06 pm on April 15, 2025: contributor

   Document the requirements around the NetEventsInterface’s methods and the lifetime of CNode objects in CConnman::m_nodes.
2. 
   DrahtBot commented at 4:06 pm on April 15, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32278.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	w0xlt

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32394 (net: make m_nodes_mutex non-recursive by vasild)
   • #32015 (net: replace manual reference counting of CNode with shared_ptr by vasild)
   • #30988 (Split CConnman by vasild)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Docs on Apr 15, 2025
4. vasild force-pushed on Apr 16, 2025
5. vasild force-pushed on Apr 16, 2025
6. doc: better document NetEventsInterface and the deletion of "CNode"s

   Document the requirements around the `NetEventsInterface`'s methods and
   the lifetime of `CNode` objects in `CConnman::m_nodes`.

   e44e669378
7. 
   vasild commented at 4:16 am on April 16, 2025: contributor

   1091faf8ff...0f26ac35e2 and 0f26ac35e2...e44e669378: further elaborate how we safely access CNodes in m_nodes and how we avoid CNode destruction while it is being referenced.
8. 
   w0xlt commented at 5:52 pm on April 17, 2025: contributor

   ACK https://github.com/bitcoin/bitcoin/pull/32278/commits/e44e669378f2c63b09eda68797039b73047febff

   Clear improvement in documentation.

9. DrahtBot added the label CI failed on Apr 28, 2025
10. DrahtBot removed the label CI failed on May 2, 2025
11. achow101 requested review from laanwj on Oct 22, 2025
12. achow101 requested review from theuni on Oct 22, 2025
13. achow101 requested review from sipa on Oct 22, 2025
14. achow101 requested review from ajtowns on Oct 22, 2025
15. in src/net.h:1475 in e44e669378

    1470+     * CNode objects keep a reference count that is used to avoid a destruction
    1471+     * while the object is used. All users of elements from `m_nodes` must either
    1472+     * do so while holding `m_nodes_mutex` or by incrementing the reference before
    1473+     * use and decrement it after use. To make sure that a new reference is not added
    1474+     * to a no-referenced CNode just before it is destroyed we use the following
    1475+     * mechanism (without a help from the standard library):
    

    ---

    ---

    

    laanwj commented at 1:48 pm on October 23, 2025:

    What do you mean with “(without a help from the standard library)”?

    ---

    

    ajtowns commented at 6:03 pm on October 29, 2025:

    I’d phrase this as:

    Destroying objects from this list works as follows (see DisconnectNodes(), DeleteNode()):

    • we will close the socket, release locks, and reset global info relating to the connection
    • the node will be moved from m_nodes to m_nodes_disconnected, preventing future calls to ProcessMessages() and SendMessages() for this node, and decrementing the reference count
    • we will wait for GetRefCount() to return <= 0 indicating there are no other references to this node in use (and hence no current calls to ProcessMessages or SendMessages)
    • the node will be removed from m_nodes_disconnected
    • FinalizeNode() will be called to indicate the node is about to be destroyed
    • the CNode is deleted
16. in src/net.h:1006 in e44e669378

    1001@@ -1002,18 +1002,31 @@ class CNode
    1002 };
    1003 
    1004 /**
    1005- * Interface for message handling
    1006+ * Interface for message handling.
    1007+ * For a given `CNode`, the methods must be called in the following order:
    

    ---

    ---

    

    ajtowns commented at 5:51 pm on October 29, 2025:

    I think this would be better phrased as requirements for implementers of the interface, rather than the user of the interface. ie: “For a given CNode, the methods will be called in the following order:”. (Both CNode and NetEventsInterface are interfaces the net module offers, so the appropriate documentation is how they work and what you need to do to implement NetEventsInterface, not how net.cpp needs to be implemented)

Contributors
vasild DrahtBot w0xlt laanwj ajtowns

Review Requested
theuni sipa

Labels
Docs