refactoring: Rename LockAnnotation to LockAssertion and add run-time check to it #16034

1. 
   practicalswift commented at 11:45 am on May 16, 2019: contributor

   LockAnnotation lock(mutex); is a guarantee to the compiler thread-analysis that mutex is locked (when it couldn’t be determined otherwise).

   Before this PR it was possible to make the mistake of adding a LockAnnotation where the correct mutex is not held. This in turn makes the thread-analysis reasoning being based on incorrect premises.

   This PR adds an assertion in the LockAnnotation ctor which checks that the guarantees given by us at compile-time are held also in practice (ifdef DEBUG_LOCKORDER).

   Issues like the one described in #16028 will be discovered immediately with this PR merged.

   Changes in this PR:

   • Move LockAnnotation from threadsafety.h (imported code) to sync.h (our code)
   • Move LockAnnotation in wallet_tests to make it reflect the truth
   • Make sure the compile-time locking promises given via LockAnnotation:s hold also in practice at runtime (ifdef DEBUG_LOCKORDER)
   • Rename LockAnnotation to LockAssertion
2. fanquake added the label Refactoring on May 16, 2019
3. 
   ryanofsky commented at 12:13 pm on May 16, 2019: member

   This change seems reasonable, but I don’t think LockAnnotation is a good name for this class if it can potentially assert and abort the program. Would suggest adding a scripted diff to rename LockAnnotation to LockAssert or something similar.
4. 
   practicalswift commented at 1:18 pm on May 16, 2019: contributor

   @ryanofsky Thanks for the quick review! I’ve now added a scripted-diff which renames LockAnnotation to LockAssertion. Please re-review :-)
5. 
   promag commented at 2:26 pm on May 16, 2019: member

   Please update OP with the rename to LockAssertion.
6. practicalswift renamed this:
   Add assertion to make sure the LockAnnotation guarantees we give are truthful (ifdef DEBUG_LOCKORDER)
   Add assertion to make sure the LockAnnotation guarantees we give are truthful (ifdef DEBUG_LOCKORDER). Rename LockAnnotation to LockAssertion.
   on May 16, 2019
7. 
   practicalswift commented at 2:49 pm on May 16, 2019: contributor

   @promag Done! Please review :-)
8. 
   DrahtBot commented at 4:15 pm on May 16, 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:

   • #15921 (Tidy up ValidationState interface by jnewbery)
   • #15713 (refactor: Replace chain relayTransactions/submitMemoryPool by higher method by ariard)

   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.

9. ryanofsky approved
10. 
    ryanofsky commented at 8:52 pm on May 16, 2019: member

    utACK last 4 commits of 8e81c1a87b45fea00bc0719569bd98ed61c1f6f4
11. Move LockAnnotation from threadsafety.h (imported code) to sync.h (our code) cc2588579c
12. Move LockAnnotation to make it reflect the truth 3a809446b3
13. Make sure the compile-time locking promises given via LockAnnotation:s hold also in practice at runtime (ifdef DEBUG_LOCKORDER) de9b5dbca3
14. scripted-diff: Rename LockAnnotation to LockAssertion

    -BEGIN VERIFY SCRIPT-
    git grep -l LockAnnotation | xargs sed -i 's/LockAnnotation/LockAssertion/'
    -END VERIFY SCRIPT-

    9f85e9cb3d
15. practicalswift force-pushed on May 17, 2019
16. 
    practicalswift commented at 11:45 am on May 17, 2019: contributor

    Rebased! Please re-review :-)
17. ryanofsky approved
18. 
    ryanofsky commented at 5:29 pm on May 23, 2019: member

    utACK 9f85e9cb3d687862128ddf464d2bc2462b8627f0. No changes at all since last review except clean rebase after base PR #16033 was merged
19. MarcoFalke renamed this:
    Add assertion to make sure the LockAnnotation guarantees we give are truthful (ifdef DEBUG_LOCKORDER). Rename LockAnnotation to LockAssertion.
    refactoring: Rename LockAnnotation to LockAssertion and add run-time check to it
    on May 23, 2019
20. MarcoFalke merged this on May 23, 2019
21. MarcoFalke closed this on May 23, 2019

    ---

22. MarcoFalke referenced this in commit 65c4bbe629 on May 23, 2019
23. sidhujag referenced this in commit 7fb5ee1cab on May 24, 2019
24. deadalnix referenced this in commit 08a7b78cc4 on May 27, 2020
25. deadalnix referenced this in commit 1250f70c37 on May 28, 2020
26. practicalswift deleted the branch on Apr 10, 2021
27. Munkybooty referenced this in commit 49329d863a on Feb 6, 2022
28. Munkybooty referenced this in commit d33ccc71ee on Feb 15, 2022
29. Munkybooty referenced this in commit 0f222b40e0 on Feb 18, 2022
30. Munkybooty referenced this in commit e0b084d9fd on Feb 18, 2022
31. Munkybooty referenced this in commit ee28ba8f19 on Feb 18, 2022
32. Munkybooty referenced this in commit 26144dffac on Apr 21, 2022
33. Munkybooty referenced this in commit cb21e05f09 on Apr 26, 2022
34. Munkybooty referenced this in commit 079827f80e on May 4, 2022
35. Munkybooty referenced this in commit f99bac35ad on May 10, 2022
36. Munkybooty referenced this in commit 50daf4dce9 on May 17, 2022
37. malbit referenced this in commit 72b925f69a on Aug 7, 2022
38. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift ryanofsky promag DrahtBot

Labels
Refactoring