[SSSD] [PATCH] Don't terminate the same connection twice
Jakub Hrozek
jhrozek at redhat.com
Wed Sep 5 10:06:22 UTC 2012
On Wed, Sep 05, 2012 at 11:08:36AM +0200, Jan Zelený wrote:
> Dne úterý 04 září 2012 18:34:23, Jakub Hrozek napsal(a):
> > The sdap_id_release_conn_data function is already called inside
> > sdap_id_op_hook_conn_data() so the net result is a double free. I can't
> > think of a reason when you'd want to call release_conn_data but not
> > op_hook_conn_data..
> >
> > This hunk was changed during the work on the primary servers. I verified
> > that primary/backup server reconnection logic still works and the
> > connection to the backup server is properly closed.
> >
> > Jan, what was the reason behind the hunk?
>
> This hunk is not my best work as I can see in retrospective ;-) You can
> certainly delete it as hotfix.
>
> The main point of this code was to prevent deleting the connection every time
> operation ends. The patch introducing the disconnection flag should be solving
> two major things:
>
> Don't accept new operations for the connection that is dicsonnecting. This is
> solved in another part of the code.
>
> Don't free the connection immediatelly when you detect it's not working.
> Rather mark it as disconnecting and let all operations on that connection
> finish (probably with an error). Only the last finished operation should be
> deleting the connection. Otherwise you will sometimes get hanging operations.
> IIRC, that's what I wanted to solve by this hunk of the code. I probably
> didn't notice the code deleting connection every time the operation ends.
>
I pushed the patch to get rid of the segfault and filed
https://fedorahosted.org/sssd/ticket/1507 to track the above.
Given that it is hard to follow the code even for the original
developer, I think the whole module is up for some commenting and maybe
even careful refactoring.
More information about the sssd-devel
mailing list