Re: [PATCH] Close idle client connections associated with closed idle pinned connections

On 17/08/2013 12:45 p.m., Alex Rousskov wrote:
> Hello,
>
> Squid was not monitoring idle persistent connections pinned to
> servers. Squid would discover that the pinned server connection is
> closed only after receiving a new request on the idle client connection
> and trying to write that request to the server. In such cases, Squid
> propagates the pinned connection closure to the client (as it should).
>
> Chrome and, to a lesser extent, Firefox handle such races by opening a
> new connection to Squid and resending the failed [idempotent] request
> transparently to the user. However, IE usually displays an error page to
> the user.
>
> While some pconn races cannot be avoided, without monitoring idle
> pconns, Squid virtually guaranteed such a race in environments where
> origin server idle connection timeout is smaller than client/Squid
> timeouts and users are revisiting server pages in the window between
> those two timeouts.
>
> With this patch, Squid monitors idle pinned connections similar to idle
> connections in the pconn pool and closes the corresponding idle client
> connection to keep the two sides in sync (to the extent possible).
>
> The attached patches are for a v3.3-based branch and trunk.
>
>
> HTH,
>
> Alex.
>
>

Audit:

* ConnStateData::clientPinnedConnectionRead please use
!getCurrentContext() instead of using !currentobject.

* I note one side effect of this monitoring:
  If the server is behaving badly and sending bytes back to Squid during
the idle period before a new client request has been received and
disabled monitoring it will result in the connection closing.
  I am in two minds about whether that is a good thing or not.
Previously server sending whitespace would have been a form of TCP
keep-alive and ignored silently by the loose parser. But it also
provided an avenue for cache poisoning and corruption by malicious
servers if they chose to send non-whitespace.

Overall I'm okay with this going in. +1.

Amos
Received on Tue Aug 20 2013 - 09:14:52 MDT