apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tom Donovan <donov...@bellatlantic.net>
Subject Re: svn commit: r659802 - in /apr/apr-util/trunk: CHANGES misc/apr_reslist.c
Date Sat, 24 May 2008 17:38:43 GMT
Nick Kew wrote:
> On Sat, 24 May 2008 15:04:01 +0200
> Ruediger Pluem <rpluem@apache.org> wrote:
> 
>> Sorry for pointing out this that late. I just thought about it again
>> and I guess we have a leak here. I think the following two lines are
>> missing here (before and after destroy_resource):
>>
>>          reslist->ntotal--;
>>
>>> +            rv = destroy_resource(reslist, res);
>>          free_container(reslist, res);
> 
> Hmmm, good catch.
> 
> Maybe it would be better to call apr_reslist_invalidate
> at this point, and keep the relevant cleanup in one place.
> 

I don't think just calling apr_reslist_invalidate will work.

apr_reslist_invalidate assumes that the reslist is not locked; but it is locked here in 
apr_reslist_acquire when the expired resource gets destroyed.

FWIW - some testing with r659802 plus Ruediger's 2-line correction shows it works as expected
with 
dbd (MySQL) connections. Expired connections are immediately destroyed down to smax - thereafter

they are destroyed & replaced as required - but in all cases expired connections are never
returned 
by apr_reslist_acquire.  I don't see any evidence of further leaks.

-tom-


Mime
View raw message