HomePhabricator

[Avalanche] Gather INVs before entering critical section for cs_vNodes

Description

[Avalanche] Gather INVs before entering critical section for cs_vNodes

Summary:
This fixes a lock order issue where ForNode() takes a lock on cs_vNodes
and locking cs_main to gather INVs within the callback trips deadlock detection,
since net_processing takes those same locks in the reverse order in another thread.

This does not suffer from the same flaw as D5692 in that it doesn't hold
an excessively long lock on cs_main. However it comes with a drawback of
gathering INVs before knowing if the peer node is still connected. I attempted
to mitigate this by making sure INVs are only gathered once. That way if one node
drops connection and another node is suitable, we don't need to take locks on cs_main twice.

Test Plan:

cmake -GNinja -DCMAKE_BUILD_TYPE=Debug ..
for i in {1..20}; do ./test/functional/test_runner.py abc-p2p-ava* ; done

There's still some rarer flaky behavior in this test, but none of the failures should
indicate a deadlock in debug.log.

Reviewers: #bitcoin_abc, deadalnix, jasonbcox

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

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

Details

Provenance
jasonbcoxAuthored on Apr 10 2020, 07:12
FabienCommitted on Apr 10 2020, 15:51
jasonbcoxPushed on Apr 10 2020, 15:53
Reviewer
Restricted Project
Differential Revision
D5693: [Avalanche] Gather INVs before entering critical section for cs_vNodes
Parents
rSTAGING28b138c1a9ba: Remove uses of chainActive and mapBlockIndex in wallet code
Branches
Unknown
Tags
Unknown