Replace RecursiveMutex cs_addrLocal with Mutex, and rename it #24108

1. 
   w0xlt commented at 9:57 am on January 20, 2022: contributor

   This PR is related to #19303 and gets rid of the RecursiveMutex cs_addrLocal.
2. scripted-diff: rename cs_addrLocal -> m_addr_local_mutex

   -BEGIN VERIFY SCRIPT-
   sed -i 's/cs_addrLocal/m_addr_local_mutex/g' -- $(git grep --files-with-matches 'cs_addrLocal')
   -END VERIFY SCRIPT-

   c4a31ca267
3. DrahtBot added the label P2P on Jan 20, 2022
4. shaavan approved
5. 
   shaavan commented at 1:59 pm on January 20, 2022: contributor

   ACK 63ca568f484a6b36b9485165c4932938f0602b36

   • The new name of cs_addrLocal -> m_addr_local_mutex is an improvement.
   • I checked that m_addr_local_mutex is being used at two places in the codebase. These instances have been addressed by Asserting Lock not held before locking the mutex. So I think it’s safe to convert m_addr_local_mutex from RecursiveMutex to Mutex.
6. 
   hebasto commented at 3:28 pm on January 20, 2022: member

   Concept ACK.

   Adding AssertLockNotHeld(m_addr_local_mutex) macros should be combined with adding thread safety annotations LOCKS_EXCLUDED(m_addr_local_mutex) in the src/net.h. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization

   Also, for the safe commit history, “refactor: replace RecursiveMutex m_addr_local_mutex with Mutex” commit should follow “p2p: add assertions for m_addr_local_mutex” one.

7. p2p: add assertions and negative TS annotations for m_addr_local_mutex 93609c1dfa
8. refactor: replace RecursiveMutex `m_addr_local_mutex` with Mutex dec787d8ac
9. w0xlt force-pushed on Jan 20, 2022
10. hebasto approved
11. 
    hebasto commented at 9:53 pm on January 20, 2022: member

    ACK dec787d8ac2e8fb42db87431dd622bf44897bc4e, I have reviewed the code and it looks OK, I agree it can be merged.
12. shaavan approved
13. 
    shaavan commented at 11:49 am on January 21, 2022: contributor

    reACK dec787d8ac2e8fb42db87431dd622bf44897bc4e

    Changes since my last review:

    • Added Thread Safety annotation, LOCKS_EXCLUDED(m_addr_local_mutex), to the Get/Set AddrLocal() functions’ declaration.
    • Rearranged commits so that m_addr_local_mutex is converted to Mutex, after, other conditions are made right for it.
14. MarcoFalke merged this on Jan 24, 2022
15. MarcoFalke closed this on Jan 24, 2022

    ---

16. sidhujag referenced this in commit 180b45eb36 on Jan 28, 2022
17. DrahtBot locked this on Jan 24, 2023

Contributors
w0xlt shaavan hebasto

Labels
P2P