]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Simplified PeerPoolMgr by removing excessive conn closure monitoring
authorAlex Rousskov <rousskov@measurement-factory.com>
Wed, 14 Jul 2021 21:22:50 +0000 (17:22 -0400)
committerAlex Rousskov <rousskov@measurement-factory.com>
Fri, 16 Jul 2021 17:33:09 +0000 (13:33 -0400)
... 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
src/PeerPoolMgr.h

index a46d8080850f546856e2da503fa2d5080cc5dc5a..cc09b12302443823c615c7a6c390cdb28194ba13 100644 (file)
@@ -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()
 {
index 3ec3491da6108d2c04ea597ea1245eb78e56b29a..4dd7e4bc5d9e2662e1b0cd2ecd2912053b9d8eba 100644 (file)
@@ -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
 };