Bug 3556: "FD ... is not an open socket" for accept() problems (#777)

Many things could go wrong after Squid successfully accept(2)ed a socket
and before that socket was registered with Comm. During that window, the
socket is stored in a refcounted Connection object. When that object was
auto-destroyed on the error handling path, its attempt to auto-close the
socket would trigger level-1 BUG 3556 errors because the socket was not
yet opened from Comm point of view. This change eliminates that "already
in Connection but not yet in Comm" window.

The fixed BUG 3556 errors stalled affected clients and leaked their FDs.

TODO: Keeping that window closed should not require a human effort, but
achieving that goal probably requires significant changes. We are
investigating.

diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index 995da7338264493147813e698f98941c37797a4f..d4b576d21936b67702800184d685f466de21a613 100644 (file)
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -374,7 +374,13 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details)
     }
 
     Must(sock >= 0);
+
+    // 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->
+    // so we end up with a uniform "(HTTP|FTP-data|HTTPS|...) remote-ip:remote-port"
+    fd_open(sock, FD_SOCKET, "HTTP Request");
     details->fd = sock;
+
     details->remote = *gai;
 
     // lookup the local-end details of this new connection
@@ -419,10 +425,6 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details)
     }
 
     /* fdstat update */
-    // XXX : these are not all HTTP requests. use a note about type and ip:port details->
-    // so we end up with a uniform "(HTTP|FTP-data|HTTPS|...) remote-ip:remote-port"
-    fd_open(sock, FD_SOCKET, "HTTP Request");
-
     fdd_table[sock].close_file = NULL;
     fdd_table[sock].close_line = 0;