From: Amos Jeffries <yadij@users.noreply.github.com>
Date: Mon, 5 Jul 2021 14:55:29 +0000 (+0000)
Subject: Fix socket accounting for TCP accept() (#854)
X-Git-Tag: SQUID_6_0_1~317
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=72bfd2f252168d73144756139076d0aced491ced;p=thirdparty%2Fsquid.git

Fix socket accounting for TCP accept() (#854)

The incoming_sockets_accepted counter is used by Comm I/O modules to
count the number of successfully accept()ed FDs during a single
select(2) (or equivalent) scan of ready descriptors. For TCP
connections, the official code incremented that counter in the async
TcpAcceptor callbacks. Those callbacks run outside the counting Comm I/O
context, rendering counter increments useless and resulting in
under-reporting of actual acceptance activity.

Also, some successfully accepted TCP connections fire no callbacks, so
it is conceptually wrong to count accepted sockets in callbacks,
regardless of the timing problems.
---

diff --git a/src/client_side.cc b/src/client_side.cc
index ef4baff8c9..004f9c907d 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2360,8 +2360,6 @@ httpAccept(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled)
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
 
-    ++incoming_sockets_accepted;
-
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Http::NewServer(xact);
     AsyncJob::Start(srv); // usually async-calls readSomeData()
@@ -2565,7 +2563,6 @@ httpsAccept(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled) {
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
     }
-    ++incoming_sockets_accepted;
 
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Https::NewServer(xact);
diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index 9da12e526f..17639607f7 100644
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -369,6 +369,7 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details)
     }
 
     Must(sock >= 0);
+    ++incoming_sockets_accepted;
 
     // Sync with Comm ASAP so that abandoned details can properly close().
     // XXX : these are not all HTTP requests. use a note about type and ip:port details->
diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 533cb0a89b..8b015f72d1 100644
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -257,8 +257,6 @@ Ftp::Server::AcceptCtrlConnection(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled)
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
 
-    ++incoming_sockets_accepted;
-
     AsyncJob::Start(new Server(xact));
 }
 
@@ -404,7 +402,6 @@ Ftp::Server::acceptDataConnection(const CommAcceptCbParams &params)
 
     debugs(33, 4, "accepted " << params.conn);
     fd_note(params.conn->fd, "passive client ftp data");
-    ++incoming_sockets_accepted;
 
     if (!clientConnection) {
         debugs(33, 5, "late data connection?");