httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: Proxy regressions
Date Thu, 04 Nov 2010 13:01:26 GMT
Tested so +1

On Nov 4, 2010, at 3:36 AM, Ruediger Pluem wrote:

> 
> 
> On 11/04/2010 01:34 AM, Rainer Jung wrote:
>> On 04.11.2010 00:57, Graham Leggett wrote:
>>> On 03 Nov 2010, at 10:28 PM, Stefan Fritsch wrote:
>>> 
>>>> Strange, I have these problems only with prefork, not with event.
>>>> But with event I get a segfault in the reslist cleanup code.
>>>> 
>>>> Can somebody cross-check this?
>>> 
>>> This smelled like a pool lifetime issue, and looking closer it looked
>>> like we were cleaning up a brigade after we had returned the backend to
>>> the pool. The attached patch stops the crash, and it seems to run at a
>>> sensible speed again (I suspect the late cleanup of the brigade was
>>> sending the code into a spin). Can you give it a try?
> 
>> From just looking at the code this seems to be the correct thing to do.
> 
>>> 
>>> One proxy test still fails though:
>>> 
>>> t/modules/proxy.............ok 7/15# Failed test 10 in t/modules/proxy.t
>>> at line 34
>>> t/modules/proxy.............FAILED test 10
>>> Failed 1/15 tests, 93.33% okay
>>> 
>>> This fails because the first bucket of the response has been corrupted:
>>> 
>>> graham-leggetts-macbook-pro-3:httpd-test minfrin$ curl
>>> http://localhost:8536/reverse/modules/cgi/nph-102.pl
>>> sl/2.3.9-dev OpenS
>>> 
>>> I've seen this a few weeks ago and it went away, so I suspect this isn't
>>> proxy that's doing it. Need to dig further.
>> 
>> Your patch fixes the crashes for me. All tests pass if the following
>> patch is also applied. Otherwise worker hangs.
>> 
>> I don't know, whether that is the right thing to add, especially since
>> "cleared" now has a use very similar to "inreslist". The patch is an
>> addon to r1026665 where the "cleaned" flag was introduced, occassionally
>> set to "1" and tested against "0", but never reset.
>> 
>> 
>> Index: modules/proxy/proxy_util.c
>> ===================================================================
>> --- modules/proxy/proxy_util.c  (revision 1030775)
>> +++ modules/proxy/proxy_util.c  (working copy)
>> @@ -2096,6 +2096,7 @@
>> #if APR_HAS_THREADS
>>     (*conn)->inreslist = 0;
>> #endif
>> +    (*conn)->cleaned = 0;
>> 
>>     return OK;
>> }
> 
> Yeah, this is correct. As I reviewed the patch initially I thought the reset
> would happen through an regular apr_pcalloc  for conn, but this is not the case.
> So the reset is the correct thing to do here.
> 
> Regards
> 
> RĂ¼diger
> 


Mime
View raw message