HomePhabricator

locks: Annotate CTxMemPool::check to require cs_main

Description

locks: Annotate CTxMemPool::check to require cs_main

Summary:
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 "core#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.

This is a backport of core#20972

Depends on D10842

Test Plan:
With TSAN:
ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10844

Details

Provenance
Carl Dong <contact@carldong.me>Authored on Jan 20 2021, 19:43
PiRKCommitted on Jan 19 2022, 17:21
PiRKPushed on Jan 19 2022, 17:21
Reviewer
Restricted Project
Differential Revision
D10844: locks: Annotate CTxMemPool::check to require cs_main
Parents
rABCf62674e36155: refactor: Clean up CTxMemPool initializer list
Branches
Unknown
Tags
Unknown