This patch addresses a number of code-maintenance issues and one hard-to-spot bug that has been hiding in the responder code for a long time.
The code has been changed so that we will now follow the tevent_req style for requesting data from the Data Provider. We will attach tevent_req objects to the DP request object and call tevent_req_[done| error] on them once the processing is complete.
For those requests that don't need to wait for a reply (such as midpoint cache refresh), the caller need only talloc_free() the returned tevent_req. The cache update will still continue in the background, but no notification of completion will be sent to this caller.
The major bug that this addresses was the present of two timeouts being monitored for the same request: one was set in tevent directly by the NSS responder and the other was created implicitly by the D-BUS communication. Under some hard-to-reproduce situations, these could interact negatively and result in one cancelling the other. It has now been reduced down to using ONLY the D-BUS timeout.
On Fri, Nov 18, 2011 at 12:17:42PM -0500, Stephen Gallagher wrote:
This patch addresses a number of code-maintenance issues and one hard-to-spot bug that has been hiding in the responder code for a long time.
The code has been changed so that we will now follow the tevent_req style for requesting data from the Data Provider. We will attach tevent_req objects to the DP request object and call tevent_req_[done| error] on them once the processing is complete.
For those requests that don't need to wait for a reply (such as midpoint cache refresh), the caller need only talloc_free() the returned tevent_req. The cache update will still continue in the background, but no notification of completion will be sent to this caller.
The major bug that this addresses was the present of two timeouts being monitored for the same request: one was set in tevent directly by the NSS responder and the other was created implicitly by the D-BUS communication. Under some hard-to-reproduce situations, these could interact negatively and result in one cancelling the other. It has now been reduced down to using ONLY the D-BUS timeout.
(I hope I don't break threading..I lost some mail and ended up importing sssd-devel contents from public archives)
Nack,
sssd_nss segfaults when setpwent is run:
Program received signal SIGSEGV, Segmentation fault. 0xnai000000000040f7dc in nss_cmd_setpwent_step (step_ctx=0x206a290) at src/responder/nss/nsssrv_cmd.c:1405 1405 cb_ctx->cctx = dctx->cmdctx->cctx; (gdb) print *dctx $1 = {cmdctx = 0x0, domain = 0x2067620, check_provider = false, res = 0x0, netgr = 0x0} (gdb) bt #0 0x000000000040f7dc in nss_cmd_setpwent_step (step_ctx=0x206a290) at src/responder/nss/nsssrv_cmd.c:1405 #1 0x000000000040f101 in nss_cmd_setpwent_send (mem_ctx=0x206d290, client=0x206ada0) at src/responder/nss/nsssrv_cmd.c:1316 #2 0x000000000040e7fe in nss_cmd_setpwent (cctx=0x206ada0) at src/responder/nss/nsssrv_cmd.c:1202 #3 0x0000000000421c5a in sss_cmd_execute (cctx=0x206ada0, sss_cmds=0x66ffa0) at src/responder/common/responder_cmd.c:98 #4 0x0000000000422b7e in client_recv (cctx=0x206ada0) at src/responder/common/responder_common.c:181 #5 0x0000000000423139 in client_fd_handler (ev=0x205d300, fde=0x20699d0, flags=1, ptr=0x206ada0) at src/responder/common/responder_common.c:219 #6 0x0000003aa42066b8 in epoll_event_loop (tvalp=0x7fffb53d6d90, std_ev=0x205d3c0) at ../tevent_standard.c:309 #7 std_event_loop_once (ev=<optimized out>, location=<optimized out>) at ../tevent_standard.c:548 #8 0x0000003aa42039f0 in _tevent_loop_once (ev=0x205d300, location=0x468817 "src/util/server.c:570") at ../tevent.c:494 #9 0x0000003aa4203b7b in tevent_common_loop_wait (ev=0x205d300, location=0x468817 "src/util/server.c:570") at ../tevent.c:595 #10 0x00000000004492e8 in server_loop (main_ctx=0x205e470) at src/util/server.c:570 #11 0x0000000000407c91 in main (argc=1, argv=0x7fffb53d70e8) at src/responder/nss/nsssrv.c:378
On Mon, 2011-11-28 at 15:01 +0100, Jakub Hrozek wrote:
On Fri, Nov 18, 2011 at 12:17:42PM -0500, Stephen Gallagher wrote:
This patch addresses a number of code-maintenance issues and one hard-to-spot bug that has been hiding in the responder code for a long time.
The code has been changed so that we will now follow the tevent_req style for requesting data from the Data Provider. We will attach tevent_req objects to the DP request object and call tevent_req_[done| error] on them once the processing is complete.
For those requests that don't need to wait for a reply (such as midpoint cache refresh), the caller need only talloc_free() the returned tevent_req. The cache update will still continue in the background, but no notification of completion will be sent to this caller.
The major bug that this addresses was the present of two timeouts being monitored for the same request: one was set in tevent directly by the NSS responder and the other was created implicitly by the D-BUS communication. Under some hard-to-reproduce situations, these could interact negatively and result in one cancelling the other. It has now been reduced down to using ONLY the D-BUS timeout.
(I hope I don't break threading..I lost some mail and ended up importing sssd-devel contents from public archives)
Nack,
sssd_nss segfaults when setpwent is run:
Thanks, I made a bad assumption in the enumeration code that dctx was available. I'm now carrying the cli_ctx explicitly.
New patch attached.
On Mon, Nov 28, 2011 at 11:28:18AM -0500, Stephen Gallagher wrote:
On Mon, 2011-11-28 at 15:01 +0100, Jakub Hrozek wrote:
On Fri, Nov 18, 2011 at 12:17:42PM -0500, Stephen Gallagher wrote:
This patch addresses a number of code-maintenance issues and one hard-to-spot bug that has been hiding in the responder code for a long time.
The code has been changed so that we will now follow the tevent_req style for requesting data from the Data Provider. We will attach tevent_req objects to the DP request object and call tevent_req_[done| error] on them once the processing is complete.
For those requests that don't need to wait for a reply (such as midpoint cache refresh), the caller need only talloc_free() the returned tevent_req. The cache update will still continue in the background, but no notification of completion will be sent to this caller.
The major bug that this addresses was the present of two timeouts being monitored for the same request: one was set in tevent directly by the NSS responder and the other was created implicitly by the D-BUS communication. Under some hard-to-reproduce situations, these could interact negatively and result in one cancelling the other. It has now been reduced down to using ONLY the D-BUS timeout.
(I hope I don't break threading..I lost some mail and ended up importing sssd-devel contents from public archives)
Nack,
sssd_nss segfaults when setpwent is run:
Thanks, I made a bad assumption in the enumeration code that dctx was available. I'm now carrying the cli_ctx explicitly.
New patch attached.
Ack, just consider one change before pushing the "error:" label in check_cache() always passes ENOMEM into nss_cmd_send_error. I think it should use whatever is in ret.
On Tue, 2011-11-29 at 12:32 +0100, Jakub Hrozek wrote:
On Mon, Nov 28, 2011 at 11:28:18AM -0500, Stephen Gallagher wrote:
On Mon, 2011-11-28 at 15:01 +0100, Jakub Hrozek wrote:
On Fri, Nov 18, 2011 at 12:17:42PM -0500, Stephen Gallagher wrote:
This patch addresses a number of code-maintenance issues and one hard-to-spot bug that has been hiding in the responder code for a long time.
The code has been changed so that we will now follow the tevent_req style for requesting data from the Data Provider. We will attach tevent_req objects to the DP request object and call tevent_req_[done| error] on them once the processing is complete.
For those requests that don't need to wait for a reply (such as midpoint cache refresh), the caller need only talloc_free() the returned tevent_req. The cache update will still continue in the background, but no notification of completion will be sent to this caller.
The major bug that this addresses was the present of two timeouts being monitored for the same request: one was set in tevent directly by the NSS responder and the other was created implicitly by the D-BUS communication. Under some hard-to-reproduce situations, these could interact negatively and result in one cancelling the other. It has now been reduced down to using ONLY the D-BUS timeout.
(I hope I don't break threading..I lost some mail and ended up importing sssd-devel contents from public archives)
Nack,
sssd_nss segfaults when setpwent is run:
Thanks, I made a bad assumption in the enumeration code that dctx was available. I'm now carrying the cli_ctx explicitly.
New patch attached.
Ack, just consider one change before pushing the "error:" label in check_cache() always passes ENOMEM into nss_cmd_send_error. I think it should use whatever is in ret.
Error code changed and pushed to master.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Stephen Gallagher