httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rasmus Lerdorf <ras...@apache.org>
Subject Re: 1.3.31 regression affecting Front Page?
Date Thu, 10 Jun 2004 20:43:06 GMT
On Wed, 9 Jun 2004, Jim Jagielski wrote:
> On Jun 9, 2004, at 3:24 PM, Rasmus Lerdorf wrote:
> > I guess what we are agreeing on here is that the logic that sets 
> > keepalive
> > to 0 is faulty and that is probably where the real fix lies.
> yeah... it's pretty inconsistent. Looking at ap_set_keepalive
> even after we know the connection will be closed, we
> set keepalive to 0, for example.

Ok, I had a closer look at the flow.  There are 6 main cases I care about.  
Static, Dynamic and Error requests for configs KeepAlive=Off and 
KeepAlive=On.  Here is what happens:

With KeepAlive On
                                                                                         
                                   
For both static and dynamic requests the flow is similar.
                      
We start connection->keepalive starts at 0 at the beginning of the request
process_request_internal eventually leads to an ap_send_http_header call 
which calls ap_set_keepalive which determines that the config has 
KeepAlive on and sets connection->keepalive to 1

For an error, like a 404, it is different.  We start off with 
connection->keepalive being set to 0, process_internal_request calls 
ap_die on the error which calls ap_send_error_response which in turn calls  
ap_send_http_header which finally sets connection_keepalive to 1.  But 
this happens after ap_die checks conn->keepalive to determine whether to 
discard the request body or not.

With KeepAlive Off

The picture is as above, except ap_set_keepalive called from 
ap_send_http_header sets connection->keepalive to 0 instead of 1.  So for 
the duration of the request, before and after checking whether we are on a 
keepalive connection, connection->keepalive is 0.

The summary here is that checking connection->keepalive before 
ap_set_keepalive() has been called really doesn't make any sense.  And we 
can't just call ap_set_keepalive in ap_die before the check because it 
would end up getting called twice and there is no guard against that in 
it.  It would double-count the request in the keepalives counter.  We need 
to either call ap_set_keepalive earlier on, like in 
process_request_internal before we hit ap_die, or we need to add a 
double-call guard in it and just add a call in ap_die before the keepalive 
check.

Another alternative would be to clean up this mess which has our undecided
state indistinguishable from our disabled state.  Checking for 0 in ap_die
is only wrong because the check is before the ap_set_keepalive call.  The
meaning of that 0 changes on that call.

-Rasmus

Mime
View raw message