On Fri, 2012-01-20 at 16:20 +0100, Jakub Hrozek wrote:
I noticed that we never marked the internal account request as done so
the internal _recv function was never called and we would never free
the
internal tevent_req structure. That means the responder_ctx would grow
in size..
The attached patch marks the internal request as done when all the
callbacks are handled. The outside account_done function is then only
called for the request that triggered DP lookup and the internal
request
is freed.
Jakub,
I and Stephen discussed the patch a bit.
I'd like to NACK for the following reasons, that are not your fault, but
require addressing IMO.
To fix the real issue ASAP I would prefer a minimal patch that simply
adds the talloc_free(subreq); and does not touch the other code.
The reason to NACK is that the original code is a bit too confusing, it
should be refactored a bit to change naming as it wrongly uses the
naming convention in an unconventional situation.
The new patch would also need quite a bit of comments because otherwise
following what is going on is arduous.
If you want to take up the task of proposing both a minimilastic first
step patch and a more complete one that would be nice, but not strictly
necessary. If you do not have the time please open a new ticket for the
second part, and just do the minimum necessary to fix the leak and
nothing more.
Thanks,
Simo.
--
Simo Sorce * Red Hat, Inc * New York