On Fri, 16 Apr 2010 10:02:08 +0400
Eugene Indenbom <eindenbom(a)gmail.com> wrote:
> You said you want to be resistant to random callback freeing
the
> sdap_handle, so this is the sequence that still gives me trouble:
>
> 1. sdap_process_result is called
> 2. sdap_handle_release(final = false) is called
> 3. conncb destroyed
> 3. op->callback is called
> 4. talloc_zfree(gsh) is called
> 5. sdap_handle_destroy is called
> 6. sdap_handle_release(final = true) is called
> 7. netx op->callback is called
> 8. talloc_zfree(gsh) is called (2nd time .. not good)
>
> I don't see how you prevent this in your code, given ctx->gsh is
> zeroed only when talloc_zfree() returns.
>
This problem was one of major issues when I have started to work on
failover reconnect problem.
The complete set of my patches addressed the issue by implementing
reference counting of connection usage (see
https://fedorahosted.org/pipermail/sssd-devel/2010-April/003113.html,
patch #4). When reference counting is in place, sdap_handle is not
destroyed until all operations complete. So inner
sdap_handle_release(final = true) calls no callbacks.
So after I suggested patch #3 as a separate fix for "early free of
sdap_handle" problem, I have looked into this issue again and found
the deficiency in talloc_zfree you have mentioned above. The I have
developed a simple patch to fix it, but was holding it back for a
while waiting for this discussion to calm down.
Now I am sending this patch in separate e-mail for approval.
Ah btw, I re-checked this with Sumit and it turns out that calling
talloc_free(X) within the destructor of X is not really a problem and
will not cause a recursion. Talloc will just return an not run the
destructor again.
So the patch to talloc_zfree() is strictly not necessary,
although it probably does no harm either so I'm ok with it.
Simo.
--
Simo Sorce * Red Hat, Inc * New York