On 10/09/2012 07:43 AM, Jakub Hrozek wrote:
On Tue, Oct 02, 2012 at 03:39:19PM +0200, Pavel Březina wrote:
> See the commit message and ticket comments for more information.
> This patch introduces one of the possible solutions, but I suggest
> to consider actually changing memory hierarchy in responders.
Instead of creating the boolean flag, what about iterating over the
dp_requests table in the sss_responder_ctx_destructor, cancel the
pending requests and remove the original sss_dp_req_destructor ?
> Currently, the hierarchy looks like this:
> main_ctx->specific_ctx->rctx, where specific_ctx is one of the pam,
> nss, sudo, etc. contexts.
> Does anybody know why it is not main_ctx->rctx->specific_ctx? This
> would solve the issue as well and I personally think that it is more
Hmm, I don't actually see any reason why the responder context is
allocated below the specific context either.
Historically, I think it's just because the responder context as a
concept came much later. Originally we only had the specific contexts,
but we later refactored the common code. When we did that, we set the
hierarchy for the common code based on execution order. (The specific
context is created first, therefore we made the responder context a
child of that).
I am not aware of any particular reason that we could not reverse this
at this point.
I discussed the hierarchy some more with Pavel and the proposal is
modify the hierarchy as follows:
main_ctx --> rctx +--> dp_requests
And then set a desctructor to dp_requests so that it is freed before
specific_ctx is as destructors of the DP requests might need to access
data from the specific_ctx.
That's an overly complex solution. What's wrong with doing
main_ctx->rctx->specific_ctx->dp_requests? Yes, I realize that
dp_requests is a component of the rctx struct, but as long as we add
comments that its memory hierarchy is maintained as a child of the
specific_ctx, I think we'd be fine.
Stephen, you probably know the hierarchy of the DP requests the
what do you think?
It's been a while, and I think you touched it more recently than I have,
with the sss_dp_issue_request() stuff, but I think the above makes sense.