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 2dda24d3a6c3bb6db291b49426d770bb36880cf6..1dcfb90da0087aac3f3f55d366bdaa9a1e831f93 100644 (file)
--- 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 a8b568afe286f9cdcc21005391578a6c0f283615..29b5355ab3fdf55435c050f342bf252f6ecf22e6 100644 (file)
--- 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());
 }