Add locking annotations for RewindBlockIndex and GetNetworkHashPS. Add missing locks. #15962

1. 
   practicalswift commented at 4:36 pm on May 6, 2019: contributor

   While investigating #15948 (comment) I noticed that calling RewindBlockIndex and GetNetworkHashPS requires holding cs_main (due to accessing chainActive) without the functions being annotated as such.

   This PR simply makes these implicit locking requirements explicit and adds two missing locks.

   The only non-test caller not holding the now annotated locks was AppInitMain, but that was not a problem in practice since the RewindBlockIndex call happens before any background threads are started.

2. Add missing locking annotation for RewindBlockIndex. Add lock in AppInitMain. 12d073adf3
3. Add missing locking annotation for GetNetworkHashPS 50ee060c06
4. practicalswift renamed this:
   Add locking annotation for RewindBlockIndex (requires holding cs_main)
   Add locking annotations for RewindBlockIndex, GetNetworkHashPS and AddTx (ListCoinsTestingSetup)
   on May 6, 2019
5. practicalswift force-pushed on May 6, 2019
6. practicalswift renamed this:
   Add locking annotations for RewindBlockIndex, GetNetworkHashPS and AddTx (ListCoinsTestingSetup)
   Add locking annotations for RewindBlockIndex and GetNetworkHashPS. Add missing locks.
   on May 6, 2019
7. Add missing lock in AddTx (ListCoinsTestingSetup) 891be7a891
8. practicalswift force-pushed on May 6, 2019
9. 
   jamesob commented at 5:18 pm on May 6, 2019: member

   utACK https://github.com/bitcoin/bitcoin/pull/15962/commits/891be7a891b676e82b8620e5c73745665e365659
10. 
    promag commented at 6:00 pm on May 6, 2019: member

    Failed with

    0 node2 stderr Assertion failed: lock cs_main held in validation.cpp:2652; locks held:
    1cs_main init.cpp:1593 (in thread init) 
    

11. DrahtBot added the label Mining on May 6, 2019
12. DrahtBot added the label RPC/REST/ZMQ on May 6, 2019
13. DrahtBot added the label Tests on May 6, 2019
14. DrahtBot added the label Validation on May 6, 2019
15. DrahtBot added the label Wallet on May 6, 2019
16. 
    DrahtBot commented at 7:58 pm on May 6, 2019: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15948 (refactor: rename chainActive by jamesob)
    • #15931 (Remove GetDepthInMainChain dependency on locked chain interface by ariard)
    • #15855 ([refactor] interfaces: Add missing LockAnnotation for cs_main by MarcoFalke)
    • #13582 (Extract AppInitLoadBlockIndex from AppInitMain by Empact)

    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.

17. practicalswift closed this on May 6, 2019

    ---

18. 
    practicalswift commented at 9:08 pm on May 6, 2019: contributor

    @promag Ouch! Thanks! Looks like we’re missing a few LOCKS_EXCLUDED annotations… Will rework - closing for now.
19. practicalswift deleted the branch on Apr 10, 2021
20. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift jamesob promag DrahtBot

Labels
Wallet Tests RPC/REST/ZMQ Mining Validation