[net] Thread safety annotations in net_processing#13423
Merged
laanwj merged 2 commits intobitcoin:masterfrom Jul 26, 2018
Merged
[net] Thread safety annotations in net_processing#13423laanwj merged 2 commits intobitcoin:masterfrom
laanwj merged 2 commits intobitcoin:masterfrom
Conversation
skeees
force-pushed
the
net_processing-thread-annotations
branch
from
8bea6e5 to
7bf1391
Compare
skeees
force-pushed
the
net_processing-thread-annotations
branch
5 times, most recently
from
3002c61 to
00c1bae
Compare
Contributor
|
Concept ACK. Well done in 4c0b978, improves thread safety analysis in a lot of places. |
Contributor
|
utACK 00c1bae9091447229cd09b5146ee25a357f592fe (although I'm not an expert on cs_main) |
skeees
force-pushed
the
net_processing-thread-annotations
branch
from
00c1bae to
af288e3
Compare
Contributor
Author
|
@DrahtBot - rebased |
Contributor
Note to reviewers: This pull request conflicts with the following ones:
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. |
This was referenced Jun 19, 2018
Contributor
|
reACK af288e3f09f0724390334d9902742883a9db388e |
skeees
force-pushed
the
net_processing-thread-annotations
branch
from
af288e3 to
4d3732d
Compare
skeees
force-pushed
the
net_processing-thread-annotations
branch
from
4d3732d to
6a2e1ee
Compare
Contributor
Author
|
rebased now that #13417 has been merged |
maflcko
reviewed
Jul 25, 2018
src/net_processing.cpp
Outdated
Member
There was a problem hiding this comment.
Can remove the now redundant mention of cs_main here?
maflcko
reviewed
Jul 25, 2018
src/net_processing.cpp
Outdated
Member
There was a problem hiding this comment.
Same here (and everywhere else)
maflcko
reviewed
Jul 26, 2018
|
|
||
| /** Increase a node's misbehavior score. */ | ||
| void Misbehaving(NodeId nodeid, int howmuch, const std::string& message=""); | ||
| void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main); |
Member
There was a problem hiding this comment.
nit: should prefix this with static to make sure the annotation is properly added where this function is declared.
Contributor
Author
There was a problem hiding this comment.
sorry - don't quite understand - this is also used in src/test/denialofservice_tests.cpp
wouldn't declaring static break that?
Member
There was a problem hiding this comment.
Ah, I see. Thanks for the clarification. I only built bitcoind to check my patch.
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 9, 2020
1e3bcd2 [net_processing] Add thread safety annotations (Jesse Cohen) f393a53 Annotate AssertLockHeld() with ASSERT_CAPABILITY() for thread safety analysis (Jesse Cohen) Pull request description: (note that this depends on bitcoin#13417) This commit fully annotates all globals in net_processing with clang thread safety annotations. Subsequent commits will begin transitioning some of this data away from cs_main into locks that are local to net_processing. Static thread safety analysis should it easier to verify correctness of that process. Tree-SHA512: b47aa410cb9ada21072370176aea9a74c575643fa1ee8cf1d43c8e28675eef17f33e5242ac422f840e8178e132ecb58412034c6334b68f1b57c686df80d4e8e2
LarryRuane
pushed a commit
to LarryRuane/zcash
that referenced
this pull request
Apr 26, 2021
…analysis zcash: cherry picked from commit f393a53 zcash: bitcoin/bitcoin#13423
LarryRuane
pushed a commit
to LarryRuane/zcash
that referenced
this pull request
Apr 26, 2021
zcash: cherry picked from commit 1e3bcd2 zcash: bitcoin/bitcoin#13423
LarryRuane
pushed a commit
to LarryRuane/zcash
that referenced
this pull request
May 27, 2021
…analysis zcash: cherry picked from commit f393a53 zcash: bitcoin/bitcoin#13423
LarryRuane
pushed a commit
to LarryRuane/zcash
that referenced
this pull request
May 27, 2021
zcash: cherry picked from commit 1e3bcd2 zcash: bitcoin/bitcoin#13423
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(note that this depends on #13417)
This commit fully annotates all globals in net_processing with clang thread safety annotations. Subsequent commits will begin transitioning some of this data away from cs_main into locks that are local to net_processing. Static thread safety analysis should it easier to verify correctness of that process.