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
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.
[PATCH 3/4] free request table as the first thing when deallocating rctx
Isn't the order guaranteed by the talloc itself?
[PATCH 4/4] coding style fix
Ack
On Tue, 2012-10-30 at 15:24 +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.
[PATCH 3/4] free request table as the first thing when deallocating rctx
Isn't the order guaranteed by the talloc itself?
The children are freed in arbitrary order, however all children of a specific context are freed before the parent.
Simo.
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..
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.
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.
On Thu, Mar 07, 2013 at 12:27:48PM +0100, Pavel Březina wrote:
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.
On Fri, Mar 08, 2013 at 02:55:25PM +0100, Jakub Hrozek wrote:
On Thu, Mar 07, 2013 at 12:27:48PM +0100, Pavel Březina wrote:
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.
I haven't seen any problems so far. Ack.
On Wed, Mar 20, 2013 at 01:20:25PM +0100, Jakub Hrozek wrote:
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.
I haven't seen any problems so far. Ack.
Pushed to master.
On Fri, 2012-10-19 at 12:27 +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
Nack please do not call talloc_free froma destructor, it is not safe IIRC.
Simo.
sssd-devel@lists.fedorahosted.org