talloc-2.1.12 issues with library destructor

[Lukas Slebodnik]

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 
>> 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.


>while I agree that we should work on deprecating and later removing
>talloc_autofree, it's currently used and the recent change in commit
>41b6810ba01f44537f470c806adb8686e1a39c48 in talloc which was aimed at fixing
>long standing Samba bug 7587 seems to cause this regression.
>
>We could try throwing destructor priorizy INT_MAX at the problem hoping noone
>else uses such a high value.
>

We would need to discuss that with gcc guys what is the best priority
for destructors. It would be unfortunate to rely on undefined behaviour
which could be changed in future. And we would need to be sure that
clang behaves the same.

Maybe, it would be the best not to rely on talloc_autofree_context
and deprecate this function. I don't know; that's the reason why I wrote this
mail.

LS