net: make m_nodes_mutex non-recursive #32394

1. 
   vasild commented at 12:04 pm on May 1, 2025: contributor

   The only case of a recursive lock was a nested ForNode() call to trim the size of lNodesAnnouncingHeaderAndIDs to <= 3. This need not be nested, so take it out.

   Before:

   0fornode(newnode)
   1    if (size >= 3)
   2        fornode(front) handle removal of front
   3        pop front
   4    push back newnode
   

   After:

   0fornode(newnode)
   1    push back newnode
   2if (size > 3)
   3    fornode(front) handle removal of front
   4    pop front
   

   lNodesAnnouncingHeaderAndIDs is protected by cs_main which is locked during the entire operation.

   Partially resolves: #19303

   ---

   This PR included #32326 (first 3 commits in this PR). That PR was merged first, so the size of this was reduced.

2. 
   DrahtBot commented at 12:04 pm on May 1, 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/32394.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	hebasto

   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:

   • #32983 (rpc: refactor: use string_view in Arg/MaybeArg by stickies-v)
   • #32278 (doc: better document NetEventsInterface and the deletion of “CNode"s by vasild)
   • #32065 (i2p: make a time gap between creating transient sessions and using them by vasild)
   • #32015 (net: replace manual reference counting of CNode with shared_ptr by vasild)
   • #30988 (Split CConnman by vasild)
   • #29415 (Broadcast own transactions only via short-lived Tor or I2P connections 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 P2P on May 1, 2025
4. 
   Bonobirho99 commented at 1:21 pm on May 1, 2025: none

   .
5. 
   hebasto commented at 2:18 pm on May 1, 2025: member

   Partially resolves: #19303

   Concept ACK on that.

6. DrahtBot added the label Needs rebase on May 20, 2025
7. vasild force-pushed on May 21, 2025
8. 
   vasild commented at 11:33 am on May 21, 2025: contributor

   98bba932bf...22d4c57a99: rebase due to conflicts
9. DrahtBot removed the label Needs rebase on May 21, 2025
10. DrahtBot added the label CI failed on Jul 30, 2025
11. DrahtBot removed the label CI failed on Jul 30, 2025
12. DrahtBot added the label Needs rebase on Sep 17, 2025
13. vasild force-pushed on Sep 29, 2025
14. 
    vasild commented at 1:00 pm on September 29, 2025: contributor

    22d4c57a99...9a49877f26: rebase due to conflicts
15. DrahtBot removed the label Needs rebase on Sep 29, 2025
16. DrahtBot added the label Needs rebase on Oct 1, 2025
17. 
    hebasto commented at 10:53 am on October 2, 2025: member

    Time to rebase once more?
18. vasild force-pushed on Oct 2, 2025
19. 
    vasild commented at 10:59 am on October 2, 2025: contributor

    9a49877f26...aaa75c1a41: rebase due to conflicts, also remove the first 3 commits from where because they were merged via #32326. Now this PR is just one commit. Also clarify a bit the commit message and the PR description.
20. DrahtBot removed the label Needs rebase on Oct 2, 2025
21. DrahtBot added the label Needs rebase on Oct 6, 2025
22. net: make m_nodes_mutex non-recursive

    The only case of a recursive lock was a nested `ForNode()` call to trim
    the size of `lNodesAnnouncingHeaderAndIDs` to `<= 3`. This need not be
    nested, so take it out.
    
    Before:
    ```
    fornode(newnode)
        if (size >= 3)
            fornode(front) handle removal of front
            pop front
        push back newnode
    ```
    
    After:
    ```
    fornode(newnode)
        push back newnode
    if (size > 3)
        fornode(front) handle removal of front
        pop front
    ```
    
    `lNodesAnnouncingHeaderAndIDs` is protected by `cs_main` which is locked
    during the entire operation.
    
    Partially resolves: https://github.com/bitcoin/bitcoin/issues/19303

    492be23a18
23. vasild force-pushed on Oct 7, 2025
24. 
    vasild commented at 9:08 am on October 7, 2025: contributor

    aaa75c1a41...492be23a18: rebase due to conflicts
25. DrahtBot removed the label Needs rebase on Oct 7, 2025

Contributors
vasild DrahtBot Bonobirho99 hebasto l0rinc

Labels
P2P