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 cc000684356d8e8dbd945c6126291b74c48daa07..1d3b2d93484fafaaae9d5f91f3d9fb1547f4e715 100644 (file)
--- 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 af877a29197271c2573d57b290727c9df740a6d4..9da12e526fcc339bf0647c012ce7e634dcd08896 100644 (file)
--- 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 5b7f4c299377c1e693f2aae4605c2db23e3fe56f..1b320e6295e398433e1da43817e5c0fd22d80a0c 100644 (file)
--- 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 e06441714cca85f63dce7ea3af0c7dff85b1ebf1..533cb0a89b34458f28072d231c2b54f639d05df4 100644 (file)
--- 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 1f1236fa0160f1a9de19739744f8162e7e642893..736365b0eacfcc5c8aa49b045b084cdc0f93fc9f 100644 (file)
--- 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