talloc-2.1.12 issues with library destructor

[Lukas Slebodnik]

On (26/03/18 10:19), Jeremy Allison via samba-technical wrote:
>On Mon, Mar 26, 2018 at 10:16:50AM -0700, Jeremy Allison via samba-technical wrote:
>> On Mon, Mar 26, 2018 at 05:00:15PM +0200, Lukas Slebodnik via samba-technical wrote:
>> > On (26/03/18 16:06), Ralph Böhme wrote:
>> > >Hi Andreas,
>> > >
>> > >On Mon, Mar 26, 2018 at 03:01:56PM +0200, Andreas Schneider via samba-technical wrote:
>> > >> destructors are normally executed after the program returns from main() or 
>> > >> after exit() is called. talloc tries to clean up its null context memory and 
>> > >> you have obviously a talloc destructor defined for your kcm_data talloc 
>> > >> context!
>> > >> 
>> > >> I don't thinks this is an issue in talloc, but sssd should cleanup the memory 
f>> > >> in orderly_shutdown() before it calls exit()!
>> > >
>> > 
>> > SSSD allocated memory on context returned by talloc_autofree_context.
>> > And I assume it was used due to following parts in documentation
>> > 
>> > """
>> >  * @brief Provide a talloc context that is freed at program exit.
>> >  *
>> >  * This is a handy utility function that returns a talloc context
>> >  * which will be automatically freed on program exit. This can be used
>> >  * to reduce the noise in memory leak reports.
>> > """
>> > 
>> > And it is not clear which library destructor will be called
>> > before talloc. Therefore it might be difficult to rely on talloc_destructors
>> > releasing properly data allocated under autofree_context.
>> > 
>> > Sure, sssd can release memory before calling exit e.g.
>> >       talloc_free(talloc_autofree_context());
>> > But then we loose an advantage of relying on automatic release
>> > provided by talloc_autofree_context.
>> 
>> Do not use talloc_autofree_context. It's addition to talloc
>> was a *big* API mistake, and I'm trying to remove it from
>> Samba in call cases.
>           ^all^ cases of course.
>
>If you look at current master Samba, do:
>
>$ git grep talloc_autofree_context
>
>- the output should be very illuminating.
>

So if I understand correctly then cannot cause any problem in samba
and thus fixing https://bugzilla.samba.org/show_bug.cgi?id=7587
was not necessary.

And as it was mention in this thread, using destructor in talloc is much worse
then using atexit (which caused issues just on FreeBSD when used in shared
libraries)


What do you think about reverting 41b6810ba01f44537f470c806adb8686e1a39c48
and changing documentation for the function talloc_autofree_context that
it is recommended to used just in daemons and not in shared libraries?

IMHO, the intention mentioned in doc-comment for talloc_autofree_context
still make a sense:

"""
 * This is a handy utility function that returns a talloc context
 * which will be automatically freed on program exit. This can be used
 * to reduce the noise in memory leak reports.
"""

And it works well if it is used in daemon an not in shared library
(because atexit would be executed just once).

LS