good points, and thanks for engaging with it.
but we shouldn't have to protect the HTTP sockets as vigorously as the P2P connections. P2P, makes sense to assume everything is an attack.
to be honest this is probably more of a nit than a real concern (only realised it after reading your comment), since both approaches end up accepting the same connections in total, just in a different order.
what i had in mind is small: it just swaps which loop is on the outside. right now it is "for each listening socket, accept connections from it until either its queue is empty or the cap is hit, then move to the next socket." the alternative is "make one pass across all listening sockets, accepting at most one connection per socket per pass, and keep repeating passes until the cap is hit or no socket yields a connection." same total number of connections accepted, same end state, just a different order
it is a bit like DFS vs BFS over the list of listening sockets, where each accepted connection is a node being visited:
current (DFS-ish): drain one socket's connections fully before moving on to the next socket
socket A connections: [x][x][x][x][x][x] ..... (all accepted first)
socket B connections: [ ][ ][ ][ ][ ][ ] ..... (none accepted yet, waiting its turn)
alternative (BFS-ish): accept one connection per socket per pass, then repeat
pass 1: socket A: 1 connection accepted socket B: 1 connection accepted
pass 2: socket A: 1 connection accepted socket B: 1 connection accepted
pass 3: socket A: 1 connection accepted socket B: 1 connection accepted
...
the BFS-style version does not accept connections any faster in total, but it means one socket's backlog of pending connections can never hold up another socket's pending connections for the whole tick. at any point mid-tick, the gap between how many connections have been accepted from Socket A versus Socket B is at most one. from the outside, this makes accepting connections look more evenly spread across bound addresses, even though the total number of accept calls and the total number of connections accepted by the end of the tick are identical
<details>
<summary>the implementation i had in mind
</summary>
void HTTPServer::SocketHandlerListening(const Sock::EventsPerSock& events_per_sock) {
if (m_stop_accepting) return;
bool accepted_any = true;
// keep making passes over all listening sockets, within this single
// tick, for as long as connections are still being accepted and
// the cap has not been hit. this is what lets the server drain a
// full backlog in one tick instead of waiting for the next tick's
// sleep/wake cycle. it stops as soon as a whole pass yields
// nothing (all queues genuinely empty) or the connection cap is
// reached, whichever comes first
while (accepted_any && GetConnectionsCount() < static_cast<size_t>(m_rpcmaxconnections)) {
accepted_any = false;
// give every listening socket one accept attempt per pass, in
// order, before any socket gets a second attempt. this keeps
// the gap between how many connections each socket has had
// accepted at 'at most' one, so a socket with a large backlog
// cannot starve out(presumably) another socket's pending
// connections within the same tick - real world might play out
// different depending on how complex
//
// CAVEAT about the 'presumed starvation': in practice this
// mostly matters when more than one listening socket is active
// (e.g. multiple -rpcbind addresses). most real-world setups
// bind to just one or two addresses (default loopback
// IPv4/IPv6), so the fairness benefit here is modest for
// typical deployments and becomes more relevant only for the
// less common case of binding to several network
// interfaces/addresses at once
for (const auto& sock : m_listen) {
if (m_interrupt_net) return;
if (GetConnectionsCount() >= static_cast<size_t>(m_rpcmaxconnections)) {
break;
}
const auto it = events_per_sock.find(sock);
if (it == events_per_sock.end() || !(it->second.occurred & Sock::RecvEvent)) {
continue;
}
CService addr_accepted;
auto sock_accepted{AcceptConnection(*sock, addr_accepted)};
if (sock_accepted) {
NewSockAccepted(std::move(sock_accepted), addr_accepted);
accepted_any = true;
}
}
}
}
</details>
If a user makes so many keep-alive connections to ::1 then opens an issue about their 127.0.0.1 connections timing out, I don't even know if we would address it as a software flaw.
💯
