From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sun, 28 Nov 2021 20:49:28 +0000 (+0000)
Subject: Fix FATAL ServiceRep::putConnection exception: theBusyConns > 0 (#939)
X-Git-Tag: SQUID_6_0_1~265
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a8ac892bab446ac11f816edec53306256bad4de7;p=thirdparty%2Fsquid.git

Fix FATAL ServiceRep::putConnection exception: theBusyConns > 0 (#939)

    FATAL: check failed: theBusyConns > 0
        exception location: ServiceRep.cc(163) putConnection

Since master/v6 commit 2b6b1bc, a timeout on a ready-to-shovel
Squid-service ICAP connection was decrementing theBusyConns level one
extra time because Adaptation::Icap::Xaction::noteCommTimedout() started
calling both noteConnectionFailed() and closeConnection(). Depending on
the actual theBusyConns level, the extra decrement could result in FATAL
errors later, when putConnection() was called (for a different ICAP
transaction) with zero theBusyConns in an exception-unprotected context.

Throughout these changes, Xaction still counts the above timeouts as a
service failure. That is done by calling ServiceRep::noteFailure() from
Xaction::callException(), including in timeout cases described above.
---

diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc
index 2dda24d3a6..1dcfb90da0 100644
--- a/src/adaptation/icap/ServiceRep.cc
+++ b/src/adaptation/icap/ServiceRep.cc
@@ -112,6 +112,8 @@ void Adaptation::Icap::ServiceRep::noteFailure()
     // should be configurable.
 }
 
+// TODO: getIdleConnection() and putConnection()/noteConnectionFailed() manage a
+// "used connection slot" resource. Automate that resource tracking (RAII/etc.).
 Comm::ConnectionPointer
 Adaptation::Icap::ServiceRep::getIdleConnection(const bool retriableXact)
 {
diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index a8b568afe2..29b5355ab3 100644
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -350,7 +350,6 @@ void Adaptation::Icap::Xaction::noteCommTimedout(const CommTimeoutCbParams &)
            theService->cfg().uri << status());
     reuseConnection = false;
     assert(haveConnection());
-    theService->noteConnectionFailed("timedout");
     closeConnection();
     throw TextException("timed out while talking to the ICAP service", Here());
 }