Re: HTTP client disconnection vs ICAP disconnection

On Tue, 8 Feb 2011 15:20:55 -0500, John Craws <john.craws_at_gmail.com>
wrote:
> Hi,
>
> Thanks for your reply. I continued my analysis and I believe the
> following is the cause of the problem that I am seeing.
>
> I'm using 3.1.10.
>
> Remember that this occurs with this particular service if the client
> request (wget) is cancelled.
>
> 1. Squid downloads the response from the origin server pretty fast.
> 2. ** The ICAP service needs the complete original response before the
> modified response may be generated for the client.
> 3. The download of the original response completes and
> HttpStateData::processReplyBody() sees a COMPLETE_NONPERSISTENT_MSG
> (because of persistentConnStatus()).
> 4. serverComplete() is called.
> 5. serverComplete() calls closeServer().
> 6. HttpStateData::closeServer() calls comm_close() on the FD, ** but
> it first removes the close handler using comm_remove_close_handler()
> **.
> 7. comm_close() (_comm_close()) calls commCallCloseHandlers(), but the
> close handler has been removed.

The request architecture in Squid is formed of Jobs ("blocks" of
processing). As each finishes they should unwind sequentially until the
client FD closes at the end of the request.
What you have described above is expected when HttpStateData job which
fetches the server HTTP reply is finished. But the data may be sitting in
Squid unprocessed.

Leaving the desired state:

 client FD open
 maybe data pending in client write queue (TCP buffers)
 maybe data pending in client streams response buffers
 maybe data pending in adaptation FD read queue (TCP buffers)
 adaptation FD open
 maybe data pending in adaptation systems
 maybe data pending in client streams adaptation write buffers
 server data producer missing
 server socket closed (or detached and in pconn cache)

>
> Therefore:
>
> 1. The close handler that had been set in the HttpStateData
> constructor (HttpStateData::httpStateConnClosed()) is not called
> (because closeServer() removed it).

Good.

> 2. -> AsyncJob::deleteThis("HttpStateData::httpStateConnClosed") is not
> called.

Ambiguous whether this is needed. IIRC at this point doneAll() supposedly
should return true which would lead to (3) below.

The server side seems very twisted and does not appear to implement the
Jobs API at the leaf HttpStateData level, doing it at the generic
ServerStateData level instead.

> 3. -> AsyncJob::callEnd() is not called.

Bad.

> 4. -> ServerStateData::swanSong() is not called.

Very bad.

> 5. -> *** cleanAdaptation() is not called.
> 6. -> "delete this" in AsyncJob::callEnd() is not called.
> 7. -> The ICAP socket is not closed.
>
> If the client is not cancelled, handleAdaptationCompleted() is
> eventually called and calls cleanAdaptation() itself. No visible
> problem.
>
> With other ICAP services where cancellation occurs while the socket to
> the ICAP service is still active, squid detects that the client
> connection has been closed (FD closed) and the correct chain of
> notification/events leads to correct ICAP cleanup. No visible problem.
>

Two problems I can see that need checking and fixing if confirmed from
that:

 * whatever stream reader is present (here adaptation) needs a kick every
time serverComplete() is called. In a way that informs it there will no
more data coming but X bytes are now ready to be handled.

 * HttpStateData needs to be left in a self-shutdown state

> I'd really appreciate your help with this.
>
> Thank you,
>
> John Craws
>
> ------------
>
> On Tue, Feb 1, 2011 at 1:59 PM, Tsantilas Christos
> <chtsanti_at_users.sourceforge.net> wrote:
>> Hi John,
>>  I did some simple tests and I am not seeing this problem on trunk or
on
>> the
>> latest squid-3.1 on bzr repository. The connection to the icap server
>> always
>> closed.
>> Is it possible to be an icap server bug?
>>
>> The debug section for the squid ICAP client  is the 93. Try to enable
>> debug
>> to see if you can understand where the problem is:
>>  debug_options ALL,1 93,9
>>
>>
>> On 02/01/2011 07:58 PM, John Craws wrote:
>>>
>>> Hi,
>>>
>>> Thanks for your reply. However, I should have added that:
>>>
>>> 1. I do not see FDs reused if the request is aborted. I see them
>>> reused if the request completes.
>>> 2. This is the case even with 'icap_persistent_connections off'.
>>>
>>> Thanks,
>>>
>>> John Craws
>>>
>>> ----------------
>>>
>>> On 01/02/11 09:04, John Craws wrote:
>>>
>>>     Hello,
>>>
>>>     I am going through the source code of 3.1.9/3.1.10 trying to find
>>> the
>>>     cause of a problem I am seeing with a squid instance configured to
>>> use
>>>     an ICAP service.
>>>
>>>     The setup works fine, but if the client (wget) is disconnected
>>> while
>>>     the response is being transferred (CTRL-C while receiving
>>> relatively
>>>     large data), the ICAP socket is never disconnected. This leads to
a
>>>     situation where the ICAP service is perpetually timing out on a
>>> read
>>>     operation on that socket.
>>>
>>>     I have not seen this occur with other ICAP services.
>>>
>>>     I would definitely appreciate if someone with better knowledge of
>>> this
>>>     part of the code could help me pinpoint where the relationship
>>> between
>>>     those two is implemented.
>>>
>>> As I understand it ICAP should not be aware of the client and has an
>>> independent set of persistent connections.
>>>
>>> The client disconnecting should set the ICAP output handlers to
>>> discard the rest of the transaction and push the FD into an IdleFdList
>>> waiting for another client request to use it.
>>>
>>> Alex or Christos know more about the finer details.
>>>
>>> Amos
>>> --
>>> Please be using
>>>   Current Stable Squid 2.7.STABLE9 or 3.1.10
>>>   Beta testers wanted for 3.2.0.4
>>>
>>
>>
Received on Wed Feb 09 2011 - 00:38:21 MST