[SSSD] [PATCH] fix early free of sdap_handle

Eugene Indenbom eindenbom at gmail.com
Wed Apr 14 14:21:50 UTC 2010


On 04/14/2010 06:02 PM, Simo Sorce wrote:
> On Wed, 14 Apr 2010 17:34:39 +0400
> Eugene Indenbom<eindenbom at gmail.com>  wrote:
>
>    
>> So it looks like you do not really believe that talloc destructors
>> are always called and it's reliable to free memory in destructors. In
>> my humble opinion if memory deallocation is controlled from
>> destructor there is actually no actual need for talloc ownership.
>>      
> If you always destroy a handle in a destructor then freeing in the
> destructor is useless, you can simply make the destructor's parent the
> memory context parent.
> If you destroy conditionally in a destructor then you risk memory
> leaks, unless you have other ways to make sure the context is freed.
>
>    
That's right, but garbage collecting logic is also a special case, 
having much bigger impact on how thing internally work and what is 
allowed and what is not.

I'd rather have destroy lock with 2 simple lock/unlock methods (like in 
my patch) than maintain garbage collector queue. And with garbage 
collector once again there is no guarantee that garbage collector queue 
and garbage collector itself would not be destroyed in event loop callback.
>> You suggestion of putting sdap_handle objects on destroy queue and
>> garbage collecting them later does not sound as better alternative.
>>      
> Unfortunately nothing is ideal here. But I want to play safe and have
> the memory hierarchy right rather then have special cases all over.
> Every special case is a long term burden.
>    

Yes, there is nothing ideal. But there is one substantial difference 
between workaround and solution:
     - Workaround fixes problem by changing __other__ (client) code in 
the way that avoids triggering a bug in the faulty code.
     - Solution fixes the buggy code so it starts to work as expected 
without putting a burden on client code of following unnecessary 
restrictions.

Regards, Eugene



More information about the sssd-devel mailing list