Page MenuHomePhabricator

[net processing] Reduce cs_main scope in MaybeDiscourageAndDisconnect()
ClosedPublic

Authored by Fabien on Jan 5 2021, 14:07.

Details

Summary
The motivation for this PR is to reduce the scope of cs_main locking in
misbehavior logic. It is the first set of commits from a larger branch
to move the misbehavior data out of CNodeState and into a new struct
that doesn't take cs_main.

There are some very minor behavior changes in this branch, such as:

    Not checking for discouragement/disconnect in ProcessMessages() (and
instead relying on the following check in SendMessages())
    Checking for discouragement/disconnect as the first action in
SendMessages() (and not doing ping message sending first)
    Continuing through SendMessages() if MaybeDiscourageAndDisconnect()
doesn't disconnect the peer (rather than dropping out of SendMessages()

Backport of core PR19472.

Depends on D8782 and D8791.

Test Plan

With Clang and debug:

ninja all check-all

./contrib/teamcity/build-configurations.py build-tsan

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Jan 5 2021, 14:07
majcosta added a subscriber: majcosta.
majcosta added inline comments.
src/net_processing.h
41 ↗(On Diff #26636)

what do you think of this? the idea is trying to have clang catch if someone tries to use the old broader locking usage in the future.

This revision is now accepted and ready to land.Jan 5 2021, 19:49