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 >
