Add compile time checking for cs_main locks which we assert at run time #14444

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:move-runtime-checking-to-compile-time-checking changing 2 files +3 −3

practicalswift commented at 12:25 PM on October 9, 2018: contributor

Assert locking requirements at compile-time (EXCLUSIVE_LOCKS_REQUIRED(foo)) instead of at run-time (AssertLockHeld(…)).
practicalswift force-pushed on Oct 9, 2018
fanquake added the label Refactoring on Oct 9, 2018
DrahtBot commented at 2:47 PM on October 9, 2018: contributor
Reviewers, this pull request conflicts with the following ones:
- #13864 (validation: Document where we are intentionally ignoring bool return values from validation related functions by practicalswift)
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.
DrahtBot cross-referenced this on Oct 9, 2018 from issue validation: Document where we are intentionally ignoring bool return values from validation related functions by practicalswift
promag commented at 12:18 AM on October 10, 2018: member

@practicalswift FYI #13783 (review) maybe you should defer these changes?
fanquake commented at 2:36 AM on October 10, 2018: member

@practicalswift In a similar vein to my comment here, please remove the AssertLockHeld changes from this PR. Leaving just the EXCLUSIVE_LOCKS_REQUIRED changes.
MarcoFalke commented at 3:56 AM on October 10, 2018: member

Agree with the previous feedback, also please squash your commits.
Add compile time checking for cs_main locks which we assert at run time 0089905361
practicalswift force-pushed on Oct 10, 2018
practicalswift commented at 9:29 AM on October 10, 2018: contributor

@promag @fanquake @MarcoFalke Thanks for the quick review and the clear feedback. Feedback addressed. Please re-review.
practicalswift renamed this:
~~Assert locking requirements at compile-time instead of at run-time~~
Add compile time checking for cs_main locks which we assert at run time
on Oct 10, 2018
promag commented at 11:10 AM on October 10, 2018: member

utACK 0089905.
MarcoFalke commented at 12:33 AM on October 24, 2018: member

utACK 00899053612d389b34470be832bc0cb73d545a33
MarcoFalke merged this on Oct 24, 2018
MarcoFalke closed this on Oct 24, 2018
MarcoFalke referenced this in commit 5473e255f6 on Oct 24, 2018
practicalswift cross-referenced this on Oct 24, 2018 from issue Meta-issue: Add Clang thread safety analysis annotations by practicalswift
jasonbcox referenced this in commit f15d3c8c03 on Oct 25, 2019
jonspock referenced this in commit 2de1971f84 on Dec 27, 2019
jonspock referenced this in commit 240f99c5b8 on Dec 29, 2019
LarryRuane referenced this in commit be2c5eb052 on Apr 5, 2021
LarryRuane cross-referenced this on Apr 5, 2021 from issue Bitcoin 0.18 locking PRs by LarryRuane
practicalswift deleted the branch on Apr 10, 2021
LarryRuane referenced this in commit 633ec7dfd0 on Apr 29, 2021
LarryRuane referenced this in commit 28fd08aacd on Jun 1, 2021
PastaPastaPasta referenced this in commit 9b4571a133 on Jul 17, 2021
bitcoin locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#13129 Meta-issue: Add Clang thread safety analysis annotations #13783 validation: Pass tx pool reference into CheckSequenceLocks #14443 Remove redundant run time assertions for locks already checked at compile time