diff options
| author | John Newbery <john@johnnewbery.com> | 2020-06-20 23:23:14 -0400 |
|---|---|---|
| committer | John Newbery <john@johnnewbery.com> | 2020-06-23 08:46:05 -0400 |
| commit | e8a2822119233ade0de84f791a9e92918a3d6896 (patch) | |
| tree | 125b0bf1c4320c597a5befe4398bbabafc4cbcfd /src | |
| parent | 3556227ddd3365cfac43b307204d73058b2943f0 (diff) | |
| download | bitcoin-e8a2822119233ade0de84f791a9e92918a3d6896.tar.xz | |
[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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/net.cpp | 9 |
1 files changed, 3 insertions, 6 deletions
diff --git a/src/net.cpp b/src/net.cpp index 371fbeed59..391391a7da 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1103,12 +1103,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) { |