From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Mon, 31 May 2021 17:32:48 +0000 (+0000)
Subject: Avoid "BUG #3329: Lost orphan ..." during accept problems (#780)
X-Git-Tag: SQUID_6_0_1~328
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4831b46cd050f069dcdd5145848ff16d1127ed44;p=thirdparty%2Fsquid.git

Avoid "BUG #3329: Lost orphan ..." during accept problems (#780)

Comm::TcpAcceptor creates a Comm::Connection with an open FD. Lots of
things could go wrong while that connection object travels to its
intended owner (e.g., Ftp::Server). A Connection object abandoned during
that travel will auto-close, triggering level-4 "BUG #3329" messages.

TODO: The manual enter/leaveOrphanage() tracking is unreliable. We need
to design and implement a true connection ownership concept that does
not require such tracking. These changes highlight handover spots.

Also made a few TcpAcceptor members protected to reduce the chance of
missing a Connection recipient (i.e. TcpAcceptor subscriber). Most of
these members should be (at least) protected for other reasons as well.
---

diff --git a/src/clients/FtpClient.cc b/src/clients/FtpClient.cc
index cc00068435..1d3b2d9348 100644
--- a/src/clients/FtpClient.cc
+++ b/src/clients/FtpClient.cc
@@ -95,6 +95,7 @@ Ftp::Channel::opened(const Comm::ConnectionPointer &newConn,
     assert(aCloser != NULL);
 
     conn = newConn;
+    conn->leaveOrphanage();
     closer = aCloser;
     comm_add_close_handler(conn->fd, closer);
 }
diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index af877a2919..9da12e526f 100644
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -375,6 +375,7 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &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->enterOrphanage();
 
     details->remote = *gai;
 
diff --git a/src/comm/TcpAcceptor.h b/src/comm/TcpAcceptor.h
index 5b7f4c2993..1b320e6295 100644
--- a/src/comm/TcpAcceptor.h
+++ b/src/comm/TcpAcceptor.h
@@ -54,6 +54,7 @@ public:
     TcpAcceptor(const Comm::ConnectionPointer &conn, const char *note, const Subscription::Pointer &aSub);
     TcpAcceptor(const AnyP::PortCfgPointer &listenPort, const char *note, const Subscription::Pointer &aSub);
 
+protected:
     /** Subscribe a handler to receive calls back about new connections.
      * Unsubscribes any existing subscribed handler.
      */
@@ -80,7 +81,6 @@ public:
     /// if not the accept() will be postponed
     static bool okToAccept();
 
-protected:
     friend class AcceptLimiter;
 
 private:
diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index e06441714c..533cb0a89b 100644
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -418,6 +418,7 @@ Ftp::Server::acceptDataConnection(const CommAcceptCbParams &params)
     } else {
         closeDataConnection();
         dataConn = params.conn;
+        dataConn->leaveOrphanage();
         uploadAvailSize = 0;
         debugs(33, 7, "ready for data");
         if (onDataAcceptCall != NULL) {
diff --git a/src/servers/Server.cc b/src/servers/Server.cc
index 1f1236fa01..736365b0ea 100644
--- a/src/servers/Server.cc
+++ b/src/servers/Server.cc
@@ -29,7 +29,9 @@ Server::Server(const MasterXaction::Pointer &xact) :
     transferProtocol(xact->squidPort->transport),
     port(xact->squidPort),
     receivedFirstByte_(false)
-{}
+{
+    clientConnection->leaveOrphanage();
+}
 
 bool
 Server::doneAll() const