You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Adam Woodworth <mi...@gmail.com> on 2008/09/19 23:24:34 UTC

Re: mod_proxy race condition bug #37770

Now that we're using 2.2.9 in the field, I've noticed a problem with
r657443, or at least it's a problem for our application.

r657443 added code to shutdown the connection w/o any response with an
EOC in the case that a second (or later) request on a keepalive
connection failed to get a response from the backend.

The problem with this is that it will also do this for connections to
the backend that timed out.  For our application, we want actual
timeouts to be caught as normal and kicked back through to the client
as a 50x error.

I made a simple one line patch to our copy of httpd 2.2.9 and it seems
to work.  I just check the rc code to see if it's a timeout error and
if so skip the new code from r657443.

I thought I'd raise this to the httpd developers -- do you think it's
something that should be part of the httpd code?  Having a timeout to
the backend causing the client to re-request seems not so good.


*** mod_proxy_http.c.bak	Thu Sep 18 20:42:05 2008
--- mod_proxy_http.c	Fri Sep 19 15:39:32 2008
***************
*** 1376,1382 ****
               * seamonkey only display an empty page in this case and do
               * not do a retry.
               */
!             if (r->proxyreq == PROXYREQ_REVERSE && c->keepalives) {
                  apr_bucket *eos;

                  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
--- 1376,1385 ----
               * seamonkey only display an empty page in this case and do
               * not do a retry.
               */
!             if (r->proxyreq == PROXYREQ_REVERSE && c->keepalives &&
rc != APR_TIMEUP) {
                  apr_bucket *eos;

                  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,



On Mon, Jun 2, 2008 at 4:21 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 06/02/2008 10:11 PM, Adam Woodworth wrote:
>>>
>>> From a usage standpoint I have no real preference other than I need to
>>
>> be able to configure it from httpd.conf on a per-host basis. :)
>>
>> What is a worker specific property?
>
> All the properties you put behind ProxyPass / balancermember or via proxyset
> in a block
> like
>
> <Proxy http://www.somebackend.com/blah>
> </Proxy>
>
> Regards
>
> Rüdiger
>
>

Re: mod_proxy race condition bug #37770

Posted by Adam Woodworth <mi...@gmail.com>.
Sounds good, glad I was able to help.  I'll keep an eye on this in a
future httpd release.


On Sat, Sep 20, 2008 at 6:42 AM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 09/20/2008 12:21 PM, Nick Kew wrote:
>>
>> On 19 Sep 2008, at 23:08, Ruediger Pluem wrote:
>>
>>> On 09/19/2008 11:24 PM, Adam Woodworth wrote:
>>>>
>>>> The problem with this is that it will also do this for connections to
>>>> the backend that timed out.  For our application, we want actual
>>>> timeouts to be caught as normal and kicked back through to the client
>>>> as a 50x error.
>>>
>>> Hm. Good point. I have to think about it, because a timeout error can
>>> justify  a repeated request (like when a firewall between frontend and
>>> backend cut the connection and now the packets are simply dropped by
>>> the firewall and a fresh one would fix this).
>>
>> I think Adam is right.  The justification for the RFC-breaking connection
>> drop is IIRC that it's merely propagating the same from the backend
>> to the client.  That doesn't apply in the case of a timeout.
>>
>
> Yes, I came to the same conclusion in the meantime. The firewall drop can be
> prevented by setting keepalive to on. Furthermore the typical keepalive
> timeouts
> for http connections are way below the typical firewall timeouts. So if we
> get back APR_TIMEUP we can safely assume that the backend did not respond in
> a timely manner. Looking at his patch I think it looks basically fine.
> Hope to find some time for a closer look and a commit later on.
>
> Regards
>
> Rüdiger
>

Re: mod_proxy race condition bug #37770

Posted by Ruediger Pluem <rp...@apache.org>.

On 09/20/2008 12:21 PM, Nick Kew wrote:
> 
> On 19 Sep 2008, at 23:08, Ruediger Pluem wrote:
> 
>> On 09/19/2008 11:24 PM, Adam Woodworth wrote:
>>>
>>> The problem with this is that it will also do this for connections to
>>> the backend that timed out.  For our application, we want actual
>>> timeouts to be caught as normal and kicked back through to the client
>>> as a 50x error.
>>
>> Hm. Good point. I have to think about it, because a timeout error can
>> justify  a repeated request (like when a firewall between frontend and
>> backend cut the connection and now the packets are simply dropped by
>> the firewall and a fresh one would fix this).
> 
> I think Adam is right.  The justification for the RFC-breaking connection
> drop is IIRC that it's merely propagating the same from the backend
> to the client.  That doesn't apply in the case of a timeout.
> 

Yes, I came to the same conclusion in the meantime. The firewall drop can be
prevented by setting keepalive to on. Furthermore the typical keepalive timeouts
for http connections are way below the typical firewall timeouts. So if we
get back APR_TIMEUP we can safely assume that the backend did not respond in
a timely manner. Looking at his patch I think it looks basically fine.
Hope to find some time for a closer look and a commit later on.

Regards

Rüdiger

Re: mod_proxy race condition bug #37770

Posted by Jim Jagielski <ji...@apache.org>.
On Sep 20, 2008, at 6:21 AM, Nick Kew wrote:

>
> On 19 Sep 2008, at 23:08, Ruediger Pluem wrote:
>
>> On 09/19/2008 11:24 PM, Adam Woodworth wrote:
>>>
>>> The problem with this is that it will also do this for connections  
>>> to
>>> the backend that timed out.  For our application, we want actual
>>> timeouts to be caught as normal and kicked back through to the  
>>> client
>>> as a 50x error.
>>
>> Hm. Good point. I have to think about it, because a timeout error can
>> justify  a repeated request (like when a firewall between frontend  
>> and
>> backend cut the connection and now the packets are simply dropped by
>> the firewall and a fresh one would fix this).
>
> I think Adam is right.  The justification for the RFC-breaking  
> connection
> drop is IIRC that it's merely propagating the same from the backend
> to the client.  That doesn't apply in the case of a timeout.
>

+1


Re: mod_proxy race condition bug #37770

Posted by Nick Kew <ni...@webthing.com>.
On 19 Sep 2008, at 23:08, Ruediger Pluem wrote:

> On 09/19/2008 11:24 PM, Adam Woodworth wrote:
>>
>> The problem with this is that it will also do this for connections to
>> the backend that timed out.  For our application, we want actual
>> timeouts to be caught as normal and kicked back through to the client
>> as a 50x error.
>
> Hm. Good point. I have to think about it, because a timeout error can
> justify  a repeated request (like when a firewall between frontend and
> backend cut the connection and now the packets are simply dropped by
> the firewall and a fresh one would fix this).

I think Adam is right.  The justification for the RFC-breaking  
connection
drop is IIRC that it's merely propagating the same from the backend
to the client.  That doesn't apply in the case of a timeout.

-- 
Nick Kew

Re: mod_proxy race condition bug #37770

Posted by Ruediger Pluem <rp...@apache.org>.

On 09/19/2008 11:24 PM, Adam Woodworth wrote:
> Now that we're using 2.2.9 in the field, I've noticed a problem with
> r657443, or at least it's a problem for our application.
> 
> r657443 added code to shutdown the connection w/o any response with an
> EOC in the case that a second (or later) request on a keepalive
> connection failed to get a response from the backend.
> 
> The problem with this is that it will also do this for connections to
> the backend that timed out.  For our application, we want actual
> timeouts to be caught as normal and kicked back through to the client
> as a 50x error.

Hm. Good point. I have to think about it, because a timeout error can
justify  a repeated request (like when a firewall between frontend and
backend cut the connection and now the packets are simply dropped by
the firewall and a fresh one would fix this).
Bug me here if I forget about it.

Regards

Rüdiger


Re: mod_proxy race condition bug #37770

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 19, 2008, at 5:24 PM, Adam Woodworth wrote:

> Now that we're using 2.2.9 in the field, I've noticed a problem with
> r657443, or at least it's a problem for our application.
>
> r657443 added code to shutdown the connection w/o any response with an
> EOC in the case that a second (or later) request on a keepalive
> connection failed to get a response from the backend.
>
> The problem with this is that it will also do this for connections to
> the backend that timed out.  For our application, we want actual
> timeouts to be caught as normal and kicked back through to the client
> as a 50x error.
>
> I made a simple one line patch to our copy of httpd 2.2.9 and it seems
> to work.  I just check the rc code to see if it's a timeout error and
> if so skip the new code from r657443.
>
> I thought I'd raise this to the httpd developers -- do you think it's
> something that should be part of the httpd code?  Having a timeout to
> the backend causing the client to re-request seems not so good.
>

Agreed. Good catch.

>
> *** mod_proxy_http.c.bak	Thu Sep 18 20:42:05 2008
> --- mod_proxy_http.c	Fri Sep 19 15:39:32 2008
> ***************
> *** 1376,1382 ****
>               * seamonkey only display an empty page in this case  
> and do
>               * not do a retry.
>               */
> !             if (r->proxyreq == PROXYREQ_REVERSE && c->keepalives) {
>                  apr_bucket *eos;
>
>                  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> --- 1376,1385 ----
>               * seamonkey only display an empty page in this case  
> and do
>               * not do a retry.
>               */
> !             if (r->proxyreq == PROXYREQ_REVERSE && c->keepalives &&
> rc != APR_TIMEUP) {
>                  apr_bucket *eos;
>
>                  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>
>
>
> On Mon, Jun 2, 2008 at 4:21 PM, Ruediger Pluem <rp...@apache.org>  
> wrote:
>>
>>
>> On 06/02/2008 10:11 PM, Adam Woodworth wrote:
>>>>
>>>> From a usage standpoint I have no real preference other than I  
>>>> need to
>>>
>>> be able to configure it from httpd.conf on a per-host basis. :)
>>>
>>> What is a worker specific property?
>>
>> All the properties you put behind ProxyPass / balancermember or via  
>> proxyset
>> in a block
>> like
>>
>> <Proxy http://www.somebackend.com/blah>
>> </Proxy>
>>
>> Regards
>>
>> Rüdiger
>>
>>
>