apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r659802 - in /apr/apr-util/trunk: CHANGES misc/apr_reslist.c
Date Sat, 24 May 2008 20:11:37 GMT


On 05/24/2008 07:38 PM, Tom Donovan wrote:
> 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.

I agree here. What worries me more is the question if are not missing a call
to free_container in apr_reslist_invalidate as well.

Regarding the two lines: Do you commit Nick or should I?

Regards

RĂ¼diger


Mime
View raw message