<html>
  <head>
    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
  </head>
  <body>
<div dir="auto">Hi,<br></div><div dir="auto"><br></div><div dir="auto">we have been having some issues lately with our Squid proxies on version 6.1.<br></div><div dir="auto"><br></div><div dir="auto">We also have the patch for the REQMOD satisfaction regression installed: <a href="https://github.com/squid-cache/squid/pull/1400">https://github.com/squid-cache/squid/pull/1400</a><br></div><div dir="auto"><br></div><div dir="auto">The issue:<br></div><div dir="auto"><br></div><div dir="auto">When there is a request sent to the ICAP server and the ICAP server is replying with a modified response, this modified response, after checking through receivedWholeAdeptedReply, doesn't set a BAD_LENGTH, which usually turns into a STREAM_UNPLANNED_COMPLETE, but rather a STREAM_COMPLETE (in Http::Stream::writeComplete). This in turn calls ConnStateData::kick on the current context, which has an empty pipeline at some point. Because this case is not specifically checked and the connection is just abandoned, the socket is never removed. This causes a lot of open connections and a high amount of file descriptors for the corresponding squid processes that we frequently run into limits.<br></div><div dir="auto"><br></div><div dir="auto">As a hot fix I made the following changes in order to accommodate for this specific case:<br></div><div dir="auto"><br></div><div dir="auto">--- a/src/client_side.cc<br></div><div dir="auto">+++ a/src/client_side.cc<br></div><div dir="auto">@@ -989,6 +989,9 @@ ConnStateData::kick()<br></div><div dir="auto">     } else if (flags.readMore) {<br></div><div dir="auto">         debugs(33, 3, clientConnection << ": calling readNextRequest()");<br></div><div dir="auto">         readNextRequest();<br></div><div dir="auto">+    } else if (pipeline.empty()) {<br></div><div dir="auto">+        debugs(33, 3, "pipeline is empty - closing connection");<br></div><div dir="auto">+        clientConnection->close();<br></div><div dir="auto">     } else {<br></div><div dir="auto">         // XXX: Can this happen? CONNECT tunnels have deferredRequest set.<br></div><div dir="auto">         debugs(33, DBG_IMPORTANT, MYNAME << "abandoning " << clientConnection);<br></div><div dir="auto"><br></div><div dir="auto">This issue also seems to happen if the ICAP server has specific responses - it seems as if the STREAM_COMPLETE case happens (implying receivedWholeAdaptedReply) if there is a Connection: closed response from ICAP, rather than Connection: keep-alive. In the latter case, the connection is torn down with a STREAM_UNPLANNED_COMPLETE. The patch, setting receivedWholeAdaptedReply always to true in those cases also means that the connection is never closed and abandoned all the time if it includes REQMOD it seems, rather than just doing that if the response from ICAP implies a closed connection.<br></div><div dir="auto"><br></div><div dir="auto">I was not able to reproduce the issue with version 5.8 as of now, so this seems to be something specific to 6.1.<br></div><div dir="auto"><br></div><div dir="auto">Do you have any remarks on the change? Maybe you have some more insight on the interplay of the code or what seems to be causing the issue in detail.</div><div dir="auto"><br></div><div dir="auto">Sincerely,<br></div><div dir="auto"><br></div><div dir="auto">Leonardo Martinho<br></div>  </body>
</html>