Re: bug #94 news

On Thu, 3 Oct 2002, Adrian Chadd wrote:

> So, basically:
>
> * kick start a forward
> * client fd goes invalid for some reason (as I saw before)
> * connect() succeeds
> * we build http headers for the request
> * bewm!
>
> Does that sound accurate?

According to your description of the filedescriptor state yes. However,
there is a abort handler registered that should have triggered when the
client fd was closed..

This abort handler can be found in forward.c (see calls to store.*Abort),
and is meant to abort the pending connect if there is no client attached.

The abort handler is called by the store when the last client detaches and
it is deemed that the result we have so far is not wirth caching
according to the quick_abort_* settings.

A reply for which we have not yet even received the reply headers should
never be seen as worth keeping, which is obviously the case for a request
where we have not yet sent the request.

What we saw in Guidos trace was that the client side request had been
restarted. Maybe this is relevant to the question why the request did not
get properly aborted. In your trace we cannot easily tell if the request
was restarted or not without looking for debug output mentioning
fwdServerClosed for this request before where it crashes in http.c..

Anyway, eleminating the mem_obj->fd is a good measure. It does not belong
there. Any client information needed should be gotten direcly from the
request structure alone, and in fact is available there even today in the
client_addr field. Remember; the only use of mem_obj->fd is to find the
client address to be used in X-Forwarded-For...

Attached is a patch that tries eleminates all uses of the client fd in
http.c and therefore the need for this assert. It is still left in the
mem_obj due to another use in store.c (line 162) that I did not notice
until trying to remove the field. But the question on why this request
was not aborted when the client fd has gone invalid remains..

Regards
Henrik