Refactor memory hierarchy in sysdb #1671

sssd-bot · 2020-05-02T12:12:44Z

Cloned from Pagure issue: https://pagure.io/SSSD/sssd/issue/629

Created at 2010-09-20 17:10:45 by sgallagh
Closed as Fixed
Assigned to jzeleny

Quoted from https://fedorahosted.org/pipermail/sssd-devel/2010-September/004624.html

There are several places in the sysdb where
we're doing this wrong, because once upon a time the sysdb was an
asynchronous interface. When this was the case, it was necessary for
these functions to have a memory context, because we needed to be able
to cancel them if the parent context was canceled.

Nowadays, the proper behavior is that a synchronous function should NOT
take a talloc context unless it is returning data. Internally to the
synchronous function, this should be the ONLY memory allocated atop this
context. Any other memory allocated during the synchronous run should be
allocated atop a tmp_ctx that is a child of NULL.

The reason for this is that it makes it much easier to track down memory
leaks. Valgrind is unable to detect when memory is still in use if it's
been attached to another memory context (because it's still referenced
by a variable that is in scope).

If the memory context happens to be something long-lived (for example
it's a direct child of the responder context or nss context) then this
memory may leak on every invocation without a way to see that it happened.

However, if the temporary context for this function is allocated on
NULL, when the function exits, if tmp_ctx hasn't been fully destroyed,
there will remain a detectable memory leak that we can use to identify
the source of growth.

As discussed in that mailing list thread, we should go through the sysdb and adjust all of the public interfaces to behave in the same manner.