From: Christos Tsantilas <chtsanti@users.sourceforge.net>
Date: Tue, 28 Apr 2015 09:55:08 +0000 (+0300)
Subject: pconn_lifetime robustness fixes
X-Git-Tag: merge-candidate-3-v1~148
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=045ccffec43db8471ca2f593da89340a63ea888c;p=thirdparty%2Fsquid.git

pconn_lifetime robustness fixes

This patch changes pconn_lifetime (r13780) to abort only really idle
persistent connections (when they timeout). It removes some "extra" features
(added to pconn_lifetime during the feature review) because they break things
when aggressive timeouts is combined with picky clients. Specifically,

1. Squid closed connections with partially received requests when they
   reached pconn_lifetime limit. We should only close _idle_ connections.

2. When connecting to Squid without sending anything for longer than
   pconn_lifetime, the connection hangs if the request is sent after the
   waiting period.

3. The connection also hangs if the initial request is starting to be
   transmitted but then there is a longer pause before the request is
   completed.

Most of the above problems are easy to trigger only when using very aggressive
pconn_lifetime settings that the feature was not designed for, but they still
can be considered bugs from admins point of view. Fixes:

* Do not stop reading a partially received request when we are timing out,
  to avoid aborting that request.

* Do not set keepalive flag based on the pconn_lifetime timeout. We cannot
  predict whether some new request data is going to be read (and reset the
  idle timeout clock) before our Connection:close response is sent back.

HTTP clients are supposed to recover from such races, but some apparently
do not, especially if it is their first request on the connection.

This is a Measurement Factory project.
---

diff --git a/src/client_side.cc b/src/client_side.cc
index 7145942582..4a5454d41c 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3065,12 +3065,6 @@ ConnStateData::clientParseRequests()
         if (concurrentRequestQueueFilled())
             break;
 
-        /*Do not read more requests if persistent connection lifetime exceeded*/
-        if (Config.Timeout.pconnLifetime && clientConnection->lifeTime() > Config.Timeout.pconnLifetime) {
-            flags.readMore = false;
-            break;
-        }
-
         // try to parse the PROXY protocol header magic bytes
         if (needProxyProtocolHeader_ && !parseProxyProtocolHeader())
             break;
diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 4b76dd6a29..4737a4c7d7 100644
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1504,10 +1504,6 @@ clientReplyContext::buildReplyHeader()
             // The listening port closed because of a reconfigure
             debugs(88, 3, "listening port closed");
             request->flags.proxyKeepalive = false;
-        } else if (Config.Timeout.pconnLifetime && conn->clientConnection->lifeTime() > Config.Timeout.pconnLifetime && conn->getConcurrentRequestCount() <= 1) {
-            // The persistent connection lifetime exceeded and we are the last parsed request
-            debugs(88, 3, "persistent connection lifetime exceeded");
-            request->flags.proxyKeepalive = false;
         }
     }