On 03/07/2013 11:26 AM, Pavel Březina wrote:
>On 10/30/2012 07:21 PM, Jakub Hrozek wrote:
>>On Tue, Oct 30, 2012 at 03:24:43PM +0100, Jakub Hrozek wrote:
>>>On Fri, Oct 19, 2012 at 12:27:44PM +0200, Pavel Březina wrote:
>>>>0001: do not leak memory on failure in *_process_init()
>>>>
>>>>0002:
https://fedorahosted.org/sssd/ticket/1575
>>>>
>>>>0003: free request table as the first thing when deallocating rctx
>>>>Because we have changed the hierarchy we do not need to special case
>>>>responder shut down in dp request destructor anymore. More
>>>>information may be found in the list archive:
>>>>https://lists.fedorahosted.org/pipermail/sssd-devel/2012-October/011663.html
>>>>
>>>>
>>>>0004: just a little coding style fix
>>>
>>>>[PATCH 1/4] do not leak memory on failure in *_process_init()
>>>
>>>If sss_process_init failed, in the per-responder _process_init
>>>functions,
>>>you would free a random pointer.
>>>
>>>>[PATCH 2/4] change responder contexts hierarchy
>>>
>>>Looks good to me and sanity testing passed as well. I have applied the
>>>patches and will them on my laptop until the other comments are fixes to
>>>test them in a real environment.
>>
>>I saw a crash when the SSSD got restarted while requests were still
>>pending:
>>0x35ba5 raise /usr/lib64/libc-2.16.so
>>0x37358 abort /usr/lib64/libc-2.16.so
>>0x22e6 talloc_abort /usr/lib64/libtalloc.so.2.0.7
>>0x2314 talloc_chunk_from_ptr.part.1 /usr/lib64/libtalloc.so.2.0.7
>>0x364f talloc_get_name /usr/lib64/libtalloc.so.2.0.7
>>0x368e talloc_check_name /usr/lib64/libtalloc.so.2.0.7
>>0x1adef nss_cmd_initgroups_search /usr/libexec/sssd/sssd_nss
>>0x1bcad nss_cmd_initgroups_dp_callback /usr/libexec/sssd/sssd_nss
>>0xcb68 nsssrv_dp_send_acct_req_done /usr/libexec/sssd/sssd_nss
>>0x50b7 _tevent_req_error /usr/lib64/libtevent.so.0.9.17
>>0x386a5 sss_dp_req_destructor /usr/libexec/sssd/sssd_nss
>>0x661d _talloc_free_children_internal.isra.4
>>/usr/lib64/libtalloc.so.2.0.7
>>0x640c _talloc_free_children_internal.isra.4
>>/usr/lib64/libtalloc.so.2.0.7
>>0x640c _talloc_free_children_internal.isra.4
>>/usr/lib64/libtalloc.so.2.0.7
>>0x640c _talloc_free_children_internal.isra.4
>>/usr/lib64/libtalloc.so.2.0.7
>>0x640c _talloc_free_children_internal.isra.4
>>/usr/lib64/libtalloc.so.2.0.7
>>0x2712 _talloc_free_internal /usr/lib64/libtalloc.so.2.0.7
>>0x38df1 __run_exit_handlers /usr/lib64/libc-2.16.so
>>0x38e75 exit /usr/lib64/libc-2.16.so -
>>0x6c83f default_quit /usr/libexec/sssd/sssd_nss
>>0x6c79 tevent_common_check_signal /usr/lib64/libtevent.so.0.9.17
>>
>>Feel free to ping me for a core file..
>
>I'm resubmitting patches. Although I don't have the debugging
>information any more, I am quite confident that the third patch caused
>this trouble, so I simple removed the patch.
>
>Even though we were able to call the dp callbacks quite safely, before
>rctx was freed, we got ourselves in trouble if the callback itself
>initialized another dp request.
>
>Now the context hierarchy is switched, but we don't run dp callbacks on
>shutdown.
I forgot to commit last changes. Valid patches are attached.
The code looks good to me so I applied the patches and will keep running
them on my machine to see if I hit any problems. If not, I'll simply
push these patches when I'm back from vacation.