From 5a529ad4e160c32783aaf11f370be22693875f7a Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 14 Jul 2021 17:22:50 -0400
Subject: [PATCH] Simplified PeerPoolMgr by removing excessive conn closure
 monitoring

... and possibly even fixed branch PeerPoolMgr changes.

Since commit 25b0ce4, Security::BlindPeerConnector (or, to be more
precise, its Security::PeerConnector parent) owns the connection being
secured, including informing the requestor (here, PeerPoolMgr) about
connection closures. This removal should have been done in 25b0ce4.

Removing closure monitoring here/now allows us to easily fix
branch-added callback-related closure handling that might have been racy
with respect to connection closure notification that could have arrived
_after_ we cancelled encryptionWait for other reasons, triggering a
Must() violation.
---
 src/PeerPoolMgr.cc | 25 -------------------------
 src/PeerPoolMgr.h  |  4 ----
 2 files changed, 29 deletions(-)

diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc
index a46d808085..cc09b12302 100644
--- a/src/PeerPoolMgr.cc
+++ b/src/PeerPoolMgr.cc
@@ -45,7 +45,6 @@ PeerPoolMgr::PeerPoolMgr(CachePeer *aPeer): AsyncJob("PeerPoolMgr"),
     request(),
     connWait(),
     encryptionWait(),
-    closer(),
     addrUsed(0)
 {
 }
@@ -112,11 +111,6 @@ PeerPoolMgr::handleOpenedConnection(const CommConnectCbParams &params)
 
     // Handle TLS peers.
     if (peer->secure.encryptTransport) {
-        typedef CommCbMemFunT<PeerPoolMgr, CommCloseCbParams> CloserDialer;
-        closer = JobCallback(48, 3, CloserDialer, this,
-                             PeerPoolMgr::handleSecureClosure);
-        comm_add_close_handler(params.conn->fd, closer);
-
         AsyncCall::Pointer callback = asyncCall(48, 4, "PeerPoolMgr::handleSecuredPeer",
                                                 MyAnswerDialer(this, &PeerPoolMgr::handleSecuredPeer));
 
@@ -147,14 +141,6 @@ PeerPoolMgr::handleSecuredPeer(Security::EncryptorAnswer &answer)
 {
     encryptionWait.finish();
 
-    if (closer != NULL) {
-        if (answer.conn != NULL)
-            comm_remove_close_handler(answer.conn->fd, closer);
-        else
-            closer->cancel("securing completed");
-        closer = NULL;
-    }
-
     if (!validPeer()) {
         debugs(48, 3, "peer gone");
         if (answer.conn != NULL)
@@ -173,17 +159,6 @@ PeerPoolMgr::handleSecuredPeer(Security::EncryptorAnswer &answer)
     pushNewConnection(answer.conn);
 }
 
-void
-PeerPoolMgr::handleSecureClosure(const CommCloseCbParams &params)
-{
-    Must(closer != NULL);
-    Must(encryptionWait);
-    encryptionWait.cancel("conn closed by a 3rd party");
-    closer = NULL;
-    // allow the closing connection to fully close before we check again
-    Checkpoint(this, "conn closure while securing");
-}
-
 void
 PeerPoolMgr::openNewConnection()
 {
diff --git a/src/PeerPoolMgr.h b/src/PeerPoolMgr.h
index 3ec3491da6..4dd7e4bc5d 100644
--- a/src/PeerPoolMgr.h
+++ b/src/PeerPoolMgr.h
@@ -55,9 +55,6 @@ protected:
     /// Security::PeerConnector callback
     void handleSecuredPeer(Security::EncryptorAnswer &answer);
 
-    /// called when the connection we are trying to secure is closed by a 3rd party
-    void handleSecureClosure(const CommCloseCbParams &params);
-
     /// the final step in connection opening (and, optionally, securing) sequence
     void pushNewConnection(const Comm::ConnectionPointer &conn);
 
@@ -71,7 +68,6 @@ private:
     /// encrypts an established transport connection
     JobWait<Security::BlindPeerConnector> encryptionWait;
 
-    AsyncCall::Pointer closer; ///< monitors conn while we are securing it
     unsigned int addrUsed; ///< counter for cycling through peer addresses
 };
 
-- 
2.47.2