Introduce SockMan ("lite"): low-level socket handling for HTTP

pinheadmz commented at 6:36 pm on June 13, 2025: member

Introduces a new low-level socket manager SockMan as an abstract class with virtual functions for implementing higher-level networking protocols like HTTP. This is the next step in #31194

This is a minimal, alternative version of #30988 (“Split CConnman”) without any changes to working code (P2P is not affected). It adds a stripped-down version of the SockMan introduced in that pull request that implements only what is needed for the HTTP server implemented in #32061 (i.e. no I2P stuff and for now, no outbound connection stuff). Exclusions from the original SockMan pull request can be checked with:

0git diff vasild/sockman \
1src/common/sockman.h \
2src/common/sockman.cpp

The commit order has been flipped quite a bit because the original PR incrementally pulls logic out of net wheras this PR builds a new system from the bottom-up. Otherwise I tried to keep all the SockMan code in order so reviewers of the original PR would be familiar with it.

It also adds unit tests by introducing a SocketTestingSetup which mocks network sockets by returning a queue of DynSock from CreateSock(). HTTP server tests in #32061 will be rebased on this framework as well.

SockMan: introduce class and implement binding to listening socket

Introduce a new low-level socket managing class `SockMan`.
Unit-test it with a new class `SocketTestingSetup` which mocks
`CreateSock()` and will enable mock client I/O in future commits.

`SockMan` and `SocketTestingSetup` are designed to be generic and
reusbale for higher-level network protocol implementation and testing.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

9299d5dbc5

style: modernize the style of SockMan::BindListenPort()

It was copied verbatim from `CConnman::BindListenPort()` in the previous
commit. Modernize its variables and style and log the error messages
from the caller. Also categorize the informative messages to the "net"
category because they are quite specific to the networking layer.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

de633698a1

SockMan: implement and test AcceptConnection()

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

f6af0c8cd1

style: modernize the style of SockMan::AcceptConnection() cca484c7ce

SockMan: generate sequential Ids for each newly accepted connection

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

1deaa087cd

DrahtBot commented at 6:36 pm on June 13, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32747.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30988 (Split CConnman by vasild)
#26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

datasent -> data sent [missing space between “data” and “sent” in comment]

drahtbot_id_4_m

pinheadmz force-pushed on Jun 13, 2025

DrahtBot added the label CI failed on Jun 13, 2025

DrahtBot commented at 7:03 pm on June 13, 2025: contributor

🚧 At least one of the CI tasks failed. Task MSan, depends: https://github.com/bitcoin/bitcoin/runs/44070031695 LLM reason (✨ experimental): MemorySanitizer detected use of uninitialized memory during sockman_tests, causing CI failure.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

pinheadmz force-pushed on Jun 13, 2025

SockMan: start an I/O loop in a new thread and accept connections

Socket handling methods are copied from CConnMan:

`CConnman::GenerateWaitSockets()` goes to
`SockMan::GenerateWaitSockets()`.

`CConnman::ThreadSocketHandler()` and
`CConnman::SocketHandler()` are combined into
`SockMan::ThreadSocketHandler()`.

`CConnman::SocketHandlerListening()` goes to
`SockMan::SocketHandlerListening()`.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

2e6cdb4804

SockMan: handle connected sockets: read data from socket

`CConnman::SocketHandlerConnected()` copied to
`SockMan::SocketHandlerConnected()`.

Testing this requires adding a new feature to the SocketTestingSetup,
inserting a "request" payload into the mock client that conencts
to us.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

674a5ff8f1

SockMan: handle connected sockets: write data to socket

Sockets-touching bits from `CConnman::SocketSendData()` copied to
`SockMan::SendBytes()`.

Testing this requires adding a new feature to the SocketTestingSetup,
returning the DynSock I/O pipes from the mock socket so the recevied
data can be checked.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

3f796b69c8

SockMan: dispatch cyclical events from I/O loop

Copy from some parts of `CConnman::SocketHandlerConnected()` and
`CConnman::ThreadSocketHandler()` to:
`EventIOLoopCompletedForOne(id)` and `EventIOLoopCompletedForAll()`.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

5f7941187a

pinheadmz force-pushed on Jun 14, 2025

DrahtBot removed the label CI failed on Jun 14, 2025

Introduce SockMan (“lite”): low-level socket handling for HTTP #32747

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)