diff options
| author | MarcoFalke <falke.marco@gmail.com> | 2021-01-21 16:45:40 +0100 |
|---|---|---|
| committer | MarcoFalke <falke.marco@gmail.com> | 2021-01-21 16:45:45 +0100 |
| commit | 1f45e855095371be8f0898f7e4d8e68dcd650663 (patch) | |
| tree | 8d280132447205f958287a6e99a203f1d1a6882c /.gitignore | |
| parent | 85fee49c39cc77b07ba57e0e7cc97f1a5b7e230c (diff) | |
| parent | b3964670537d0943b8fb2d8f2ea419cbefd4835a (diff) | |
Merge #20972: locks: Annotate CTxMemPool::check to require cs_main
b3964670537d0943b8fb2d8f2ea419cbefd4835a locks: Annotate CTxMemPool::check to require cs_main (Carl Dong)
Pull request description:
```
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
calls GetSpendHeight which locks cs_main. This can potentially cause an
undesirable lock invesion since CTxMemPool's cs is supposed to be locked
after cs_main.
This does not cause us any problems right now because all callers of
CTxMemPool already lock cs_main before calling CTxMemPool::check, which
means that the LOCK(cs_main) in GetSpendHeight becomes benign.
However, it is currently possible for new code to be added which calls
CTxMemPool::check without locking cs_main (which would be dangerous).
Therefore we should make it explicit that cs_main needs to be held
before calling CTxMemPool::check.
NOTE: After all review-only assertions are removed in "#20158 |
tree-wide: De-globalize ChainstateManager", and assuming that we
keep the changes in "validation: Pass in spendheight to
CTxMemPool::check", we can re-evaluate to see if this annotation
is still necessary.
```
-----
Previous discussions:
1. https://github.com/bitcoin/bitcoin/pull/20158#discussion_r520639845
2. https://github.com/bitcoin/bitcoin/pull/20158#pullrequestreview-557117202
3. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559425521
ACKs for top commit:
jnewbery:
Code review ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a
MarcoFalke:
ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a
jonatack:
ACK b3964670537d0943b8fb2d8f2ea419cbefd4835a review and debug built, verified that `cs_main` is held by callers of `CTxMemPool::check()` in `PeerManagerImpl::ProcessOrphanTx()`, `PeerManagerImpl::ProcessMessage()`, and `CChainState::ActivateBestChainStep()`
Tree-SHA512: 4635cddb4aa1af9532bb657b2f9c4deec4568d16ba28c574eae91bb77368cd40e23c3c720a9de11cec78e7ad678a44a5e25af67f13214b86b56e777e0c35a026
Diffstat (limited to '.gitignore')
0 files changed, 0 insertions, 0 deletions