Removed branch-added try/catch fixing orphaned params.conn

The problem is real, but there are other similarly problematic places in
the official code. If we fix this one place, then we probably should fix
all other known ones. However,

* the related exceptions ought be rare (there are no known ones);
* Squid will probably recover even if the affected Connection objects
  are orphaned; and
* the appropriate fix (i.e. custom try/catch blocks) is not the right
  long-term solution -- we need a better Connection storage API that
  would do the right thing automatically.

Given the combination of these factors, I think it is best to do nothing
here/now but add the right long-term API in a dedicated branch.

diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc
index 678fda0040f755025cef18120b3c03cabdb91135..5e8d9986d1901fe82efa7271ba5a015024767f96 100644 (file)
--- a/src/PeerPoolMgr.cc
+++ b/src/PeerPoolMgr.cc
@@ -108,34 +108,21 @@ PeerPoolMgr::handleOpenedConnection(const CommConnectCbParams &params)
 
     // Handle TLS peers.
     if (peer->secure.encryptTransport) {
-        encryptTransport(params.conn);
-        return;
-    }
-
-    pushNewConnection(params.conn);
-}
-
-void
-PeerPoolMgr::encryptTransport(const Comm::ConnectionPointer &conn)
-{
-    try {
+        // XXX: Exceptions orphan params.conn
         AsyncCall::Pointer callback = asyncCall(48, 4, "PeerPoolMgr::handleSecuredPeer",
                                                 MyAnswerDialer(this, &PeerPoolMgr::handleSecuredPeer));
 
         const auto peerTimeout = peer->connectTimeout();
-        const int timeUsed = squid_curtime - conn->startTime();
+        const int timeUsed = squid_curtime - params.conn->startTime();
         // Use positive timeout when less than one second is left for conn.
         const int timeLeft = positiveTimeout(peerTimeout - timeUsed);
-        const auto connector = new Security::BlindPeerConnector(request, conn, callback, nullptr, timeLeft);
+        const auto connector = new Security::BlindPeerConnector(request, params.conn, callback, nullptr, timeLeft);
         encryptionWait.start(connector, callback);
         AsyncJob::Start(connector); // will call our callback
+        return;
     }
-    catch (...) {
-        conn->close();
-        // We could report and continue if we can recover from this failure, but
-        // it is difficult to determine/do that correctly so lets KISS for now.
-        throw;
-    }
+
+    pushNewConnection(params.conn);
 }
 
 void

diff --git a/src/PeerPoolMgr.h b/src/PeerPoolMgr.h
index 54af79973c93719982df8ca567aa686e0724e7c9..ae07c1fd1c9533fe0287f84e41516b2c03817b05 100644 (file)
--- a/src/PeerPoolMgr.h
+++ b/src/PeerPoolMgr.h
@@ -52,9 +52,6 @@ protected:
     /// Comm::ConnOpener calls this when done opening a connection for us
     void handleOpenedConnection(const CommConnectCbParams &params);
 
-    /// initiates Security::PeerConnector work on a just-established connection
-    void encryptTransport(const Comm::ConnectionPointer &);
-
     /// Security::PeerConnector callback
     void handleSecuredPeer(Security::EncryptorAnswer &answer);