net: refactor: cleanup ThreadSocketHandler #14335

pull pstratem wants to merge 4 commits into bitcoin:master from pstratem:2018-09-24-thread-handler-cleanup changing 2 files +257 −239

pstratem commented at 9:15 PM on September 26, 2018: contributor

Working towards using poll() on unix like systems.

A number of small changes designed to separate the actual socket handling from the rest of the logic in ThreadSocketHandler.

This is a simpler version of #14147
Move NotifyNumConnectionsChanged logic to private method. edb5350c32
Move DisconnectNodes logic to private method. 7479b63d91
Move InactivityCheck logic to private method. 2af9cff11a
Move SocketHandler logic to private method. 032488e6e7
DrahtBot commented at 10:27 PM on September 26, 2018: member
Reviewers, this pull request conflicts with the following ones:
- #14046 (net: Refactor message parsing (CNetMessage), adds flexibility by jonasschnelli)
- #14032 (Add p2p layer encryption with ECDH/ChaCha20Poly1305 by jonasschnelli)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
fanquake added the label P2P on Sep 26, 2018
MarcoFalke added the label Refactoring on Sep 26, 2018
ryanofsky commented at 5:26 PM on September 27, 2018: member

utACK 032488e6e7b4c2d37f3f434d575d25806ddcb0b0. PR description above should contain an actual description of the changes, though, and not just reference #14147. (This PR seems the same as #14147 except it replaces InactivityChecks with InactivityCheck and moves it below NotifyNumConnectionsChanged. I'm actually not sure why #14147 was closed and replaced instead of just updated.)
pstratem commented at 7:25 PM on September 27, 2018: contributor

@ryanofsky I wanted to simplify the changes to the point that they are as obviously correct as possible.

All of these changes are now just moving logic to functions.
DrahtBot commented at 2:20 AM on September 28, 2018: member



Coverage Change (pull 14335) Reference (master)

Lines +0.0012 % 87.0361 %

Functions +0.0252 % 84.1130 %

Branches -0.0026 % 51.5451 %
sipa commented at 7:57 PM on October 12, 2018: member

utACK 032488e6e7b4c2d37f3f434d575d25806ddcb0b0. Nice to see logic moved out of those giant blocks of code.
donaloconnor commented at 10:26 PM on October 12, 2018: contributor

utACK - looks much better 👍
meshcollider commented at 10:58 PM on October 12, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/commit/032488e6e7b4c2d37f3f434d575d25806ddcb0b0, nice refactor
laanwj commented at 3:19 PM on October 16, 2018: member

utACK caa08261d1fa1e699b4e649b644dd4f3520de755
laanwj merged this on Oct 16, 2018
laanwj closed this on Oct 16, 2018
laanwj referenced this in commit 23419e4c49 on Oct 16, 2018
MarcoFalke commented at 10:05 PM on October 23, 2018: member

post-merge utACK 032488e6e7b4c2d37f3f434d575d25806ddcb0b0
codablock referenced this in commit 3bcee4a480 on Apr 8, 2020
codablock referenced this in commit f9bf204829 on Apr 8, 2020
MarcoFalke locked this on Sep 8, 2021