HomePhabricator

[net] Make cs_inventory nonrecursive

Description

[net] Make cs_inventory nonrecursive

Summary:

[net processing] Remove PushBlockInventory and PushBlockHash

PushBlockInventory() and PushBlockHash() are functions that can
be replaced with single-line statements. This also eliminates
the single place that cs_inventory is taken recursively.

[net] Make cs_inventory a non-recursive mutex

cs_inventory is never taken recursively. Make it a non-recursive mutex.

[net] Don't try to take cs_inventory before deleting CNode

The TRY_LOCK(cs_inventory) in DisconnectNodes() is taken after the CNode
object has been removed from vNodes and when the CNode's nRefCount is
zero.

The only other places that cs_inventory can be taken are:

  • In ProcessMessages() or SendMessages(), when the CNode's nRefCount

must be >0 (see ThreadMessageHandler(), where the refcount is
incremented before calling ProcessMessages() and SendMessages()).

  • In a ForEachNode() lambda in PeerLogicValidation::UpdatedBlockTip().

ForEachNode() locks cs_vNodes and calls the function on the CNode
objects in vNodes.

Therefore, cs_inventory is never locked by another thread when the
TRY_LOCK(cs_inventory) is reached in DisconnectNodes(). Since the
only purpose of this TRY_LOCK is to ensure that the lock is not
taken by another thread, this always succeeds. Remove the check.

This is a backport of core#19347

Test Plan: ninja all check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

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

Details

Provenance
John Newbery <john@johnnewbery.com>Authored on Apr 20 2021, 16:41
PiRKCommitted on Apr 20 2021, 16:41
abc-botPushed on Apr 20 2021, 16:48
Reviewer
Restricted Project
Differential Revision
D9433: [net] Make cs_inventory nonrecursive
Parents
rABC9637276974d0: [avalanche] support MSG_AVA_PROOF in CInv::GetCommand
Branches
Unknown
Tags
Unknown