Merge #19347: [net] Make cs_inventory nonrecursive

e8a2822119 [net] Don't try to take cs_inventory before deleting CNode (John Newbery)
3556227ddd [net] Make cs_inventory a non-recursive mutex (John Newbery)
344e831de5 [net processing] Remove PushBlockInventory and PushBlockHash (John Newbery)

Pull request description:

  - Remove PushBlockInventory() and PushBlockHash(). These are one-line functions that can easy be inlined into the calling code. Doing so also allows us to eliminate the one place that cs_inventory is recursively locked.
  - Make cs_inventory a nonrecursive mutex
  - Remove a redundant TRY_LOCK of cs_inventory when deleting CNode.

ACKs for top commit:
  sipa:
    utACK e8a2822119
  MarcoFalke:
    ACK e8a2822119 🍬
  hebasto:
    re-ACK e8a2822119

Tree-SHA512: dbc721d102cdef7b5827a8f2549daf8b54f543050266999a7ea56c9f36618565b71e31ce0beb1209ba2db43d15388be173355a03fb6db8ad24e2475b145050bd
This commit is contained in:
MarcoFalke 2020-07-08 21:57:19 +02:00
commit 9f4c0a9694
No known key found for this signature in database
GPG Key ID: CE2B75697E69A548
3 changed files with 9 additions and 23 deletions

View File

@ -1110,12 +1110,9 @@ void CConnman::DisconnectNodes()
if (pnode->GetRefCount() <= 0) {
bool fDelete = false;
{
TRY_LOCK(pnode->cs_inventory, lockInv);
if (lockInv) {
TRY_LOCK(pnode->cs_vSend, lockSend);
if (lockSend) {
fDelete = true;
}
TRY_LOCK(pnode->cs_vSend, lockSend);
if (lockSend) {
fDelete = true;
}
}
if (fDelete) {

View File

@ -803,7 +803,7 @@ public:
// There is no final sorting before sending, as they are always sent immediately
// and in the order requested.
std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory);
RecursiveMutex cs_inventory;
Mutex cs_inventory;
struct TxRelay {
mutable RecursiveMutex cs_filter;
@ -982,18 +982,6 @@ public:
}
}
void PushBlockInventory(const uint256& hash)
{
LOCK(cs_inventory);
vInventoryBlockToSend.push_back(hash);
}
void PushBlockHash(const uint256 &hash)
{
LOCK(cs_inventory);
vBlockHashesToAnnounce.push_back(hash);
}
void CloseSocketDisconnect();
void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap);

View File

@ -1325,9 +1325,10 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB
}
// Relay inventory, but don't relay old inventory during initial block download.
connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) {
LOCK(pnode->cs_inventory);
if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) {
for (const uint256& hash : reverse_iterate(vHashes)) {
pnode->PushBlockHash(hash);
pnode->vBlockHashesToAnnounce.push_back(hash);
}
}
});
@ -1604,7 +1605,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c
// Trigger the peer node to send a getblocks request for the next batch of inventory
if (inv.hash == pfrom.hashContinue)
{
// Bypass PushBlockInventory, this must send even if redundant,
// Send immediately. This must send even if redundant,
// and we want it right after the last block so they don't
// wait for other stuff first.
std::vector<CInv> vInv;
@ -2664,7 +2665,7 @@ void ProcessMessage(
LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());
break;
}
pfrom.PushBlockInventory(pindex->GetBlockHash());
WITH_LOCK(pfrom.cs_inventory, pfrom.vInventoryBlockToSend.push_back(pindex->GetBlockHash()));
if (--nLimit <= 0)
{
// When this block is requested, we'll send an inv that'll
@ -4082,7 +4083,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
// If the peer's chain has this block, don't inv it back.
if (!PeerHasHeader(&state, pindex)) {
pto->PushBlockInventory(hashToAnnounce);
pto->vInventoryBlockToSend.push_back(hashToAnnounce);
LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__,
pto->GetId(), hashToAnnounce.ToString());
}