From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Tue, 6 Sep 2011 17:57:57 +0000 (-0600)
Subject: Send RST packet when closing an ICAP connection after a transaction error.
X-Git-Tag: take08~24^2~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a32c060fb1c92c9aa2f794ba86d06dc847816939;p=thirdparty%2Fsquid.git

Send RST packet when closing an ICAP connection after a transaction error.

This helps identify abnormal transaction termination at the ICAP server. The
code is from a production-running branch and was proven useful in some
environments (especially if the ICAP service needs help correctly dealing with
prematurely terminated transfers), but it is not yet clear whether the
advantages of doing this on a regular basis outweigh the overheads. If not,
we will need a squid.conf parameter to control connection closing behavior.
---

diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc
index 2cdbb59454..d4028c18e5 100644
--- a/src/adaptation/icap/ServiceRep.cc
+++ b/src/adaptation/icap/ServiceRep.cc
@@ -115,7 +115,7 @@ Adaptation::Icap::ServiceRep::getConnection(bool retriableXact, bool &reused)
 }
 
 // pools connection if it is reusable or closes it
-void Adaptation::Icap::ServiceRep::putConnection(const Comm::ConnectionPointer &conn, bool isReusable, const char *comment)
+void Adaptation::Icap::ServiceRep::putConnection(const Comm::ConnectionPointer &conn, bool isReusable, bool sendReset, const char *comment)
 {
     Must(Comm::IsConnOpen(conn));
     // do not pool an idle connection if we owe connections
@@ -124,9 +124,14 @@ void Adaptation::Icap::ServiceRep::putConnection(const Comm::ConnectionPointer &
         commUnsetConnTimeout(conn);
         theIdleConns->push(conn);
     } else {
-        debugs(93, 3, HERE << "closing pconn" << comment);
-        // comm_close will clear timeout
-        conn->close();
+        debugs(93, 3, HERE << (sendReset ? "RST" : "FIN") << "-closing " <<
+               comment);
+        // comm_close called from Connection::close will clear timeout
+        // TODO: add "bool sendReset = false" to Connection::close()?
+        if (sendReset)
+            comm_reset_close(conn);
+        else
+            conn->close();
     }
 
     Must(theBusyConns > 0);
diff --git a/src/adaptation/icap/ServiceRep.h b/src/adaptation/icap/ServiceRep.h
index f7c342469f..f39e2f4e6b 100644
--- a/src/adaptation/icap/ServiceRep.h
+++ b/src/adaptation/icap/ServiceRep.h
@@ -111,7 +111,7 @@ public:
     bool allows204() const;
     bool allows206() const;
     Comm::ConnectionPointer getConnection(bool isRetriable, bool &isReused);
-    void putConnection(const Comm::ConnectionPointer &conn, bool isReusable, const char *comment);
+    void putConnection(const Comm::ConnectionPointer &conn, bool isReusable, bool sendReset, const char *comment);
     void noteConnectionUse(const Comm::ConnectionPointer &conn);
     void noteConnectionFailed(const char *comment);
 
diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index 58d9f9d777..6a01aad1bf 100644
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -204,8 +204,11 @@ void Adaptation::Icap::Xaction::closeConnection()
         if (reuseConnection)
             disableRetries();
 
+        const bool reset = !reuseConnection &&
+            (al.icap.outcome == xoGone || al.icap.outcome == xoError);
+
         Adaptation::Icap::ServiceRep &s = service();
-        s.putConnection(connection, reuseConnection, status());
+        s.putConnection(connection, reuseConnection, reset, status());
 
         writer = NULL;
         reader = NULL;