-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
We weren't decrementing the count of in-progress authentication request child processes when they completed successfully. With this patch, we will now guarantee that the process count is accurate and that queued requests will be started when a slot is freed up.
Fixes https://fedorahosted.org/sssd/ticket/660
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/16/2010 04:09 PM, Stephen Gallagher wrote:
We weren't decrementing the count of in-progress authentication request child processes when they completed successfully. With this patch, we will now guarantee that the process count is accurate and that queued requests will be started when a slot is freed up.
Sorry, I realized after I sent it that I only moved the problem. The first patch would have caused a similar problem if the auth child experienced an error. New patch attached should be correct for both cases.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Tue, Nov 16, 2010 at 04:12:40PM -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/16/2010 04:09 PM, Stephen Gallagher wrote:
We weren't decrementing the count of in-progress authentication request child processes when they completed successfully. With this patch, we will now guarantee that the process count is accurate and that queued requests will be started when a slot is freed up.
Sorry, I realized after I sent it that I only moved the problem. The first patch would have caused a similar problem if the auth child experienced an error. New patch attached should be correct for both cases.
Can you call 'client_ctx->auth_ctx->running--;' directly after 'proxy_child_recv()' ?
bye, Sumit
Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkzi88gACgkQeiVVYja6o6PbdwCgkLZHDhz+raC96idCn+RfeLtf RFcAnAkTb8iT9+fk41JeILzoG/NwW+RL =45K8 -----END PGP SIGNATURE-----
From 499993e068bf9ad1798e384a9db18f5565d45f0e Mon Sep 17 00:00:00 2001 From: Stephen Gallagher sgallagh@redhat.com Date: Tue, 16 Nov 2010 16:05:50 -0500 Subject: [PATCH] Fix authentication queue code for proxy auth
We weren't decrementing the count of in-progress authentication request child processes when they completed successfully. With this patch, we will now guarantee that the process count is accurate and that queued requests will be started when a slot is freed up.
src/providers/proxy/proxy_auth.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/providers/proxy/proxy_auth.c b/src/providers/proxy/proxy_auth.c index 64b38cbedf30d0989f9517763e8281abb67958d8..edc35adfeb2471bbd3e005f4fdda1ab00c88caab 100644 --- a/src/providers/proxy/proxy_auth.c +++ b/src/providers/proxy/proxy_auth.c @@ -738,6 +738,19 @@ static void proxy_child_done(struct tevent_req *req) return; }
- /* Start the next auth in the queue, if any */
- client_ctx->auth_ctx->running--;
- imm = tevent_create_immediate(client_ctx->be_req->be_ctx->ev);
- if (imm == NULL) {
DEBUG(1, ("tevent_create_immediate failed.\n"));
return;
- }
- tevent_schedule_immediate(imm,
client_ctx->be_req->be_ctx->ev,
run_proxy_child_queue,
client_ctx->auth_ctx);
- /* Check if we need to save the cached credentials */ if ((pd->cmd == SSS_PAM_AUTHENTICATE || pd->cmd == SSS_PAM_CHAUTHTOK) && pd->pam_status == PAM_SUCCESS &&
-- 1.7.3.2
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/17/2010 04:22 AM, Sumit Bose wrote:
Can you call 'client_ctx->auth_ctx->running--;' directly after 'proxy_child_recv()' ?
Sure, I just moved the decrement and the creation of the immediate event to before the return value check, so now it will happen regardless of the result code. This also eliminates the code duplication in my previous patch for creating the immediate event.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Wed, Nov 17, 2010 at 02:32:39PM -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/17/2010 04:22 AM, Sumit Bose wrote:
Can you call 'client_ctx->auth_ctx->running--;' directly after 'proxy_child_recv()' ?
Sure, I just moved the decrement and the creation of the immediate event to before the return value check, so now it will happen regardless of the result code. This also eliminates the code duplication in my previous patch for creating the immediate event.
if the proxy_child fails and tevent_create_immediate(), too, proxy_reply() is not called.
bye, Sumit
Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkzkLdMACgkQeiVVYja6o6PpFwCfRfMM3Pv5jKoIv0xXGkFtn3lI ydEAniJHN2NIQVCH9vvPzezfbThmLKDG =X89X -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/17/2010 02:38 PM, Sumit Bose wrote:
On Wed, Nov 17, 2010 at 02:32:39PM -0500, Stephen Gallagher wrote: On 11/17/2010 04:22 AM, Sumit Bose wrote:
Can you call 'client_ctx->auth_ctx->running--;' directly after 'proxy_child_recv()' ?
Sure, I just moved the decrement and the creation of the immediate event to before the return value check, so now it will happen regardless of the result code. This also eliminates the code duplication in my previous patch for creating the immediate event.
if the proxy_child fails and tevent_create_immediate(), too, proxy_reply() is not called.
Good catch. However, if we have ENOMEM here (the only way tevent_create_immediate() can fail) and there are events on the queue, then one of them may not fire. However, this is a hopefully impossible situation to get into, since we're freeing req just above it. I've added comments to that effect to the patch.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
On Wed, Nov 17, 2010 at 02:58:17PM -0500, Stephen Gallagher wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/17/2010 02:38 PM, Sumit Bose wrote:
On Wed, Nov 17, 2010 at 02:32:39PM -0500, Stephen Gallagher wrote: On 11/17/2010 04:22 AM, Sumit Bose wrote:
Can you call 'client_ctx->auth_ctx->running--;' directly after 'proxy_child_recv()' ?
Sure, I just moved the decrement and the creation of the immediate event to before the return value check, so now it will happen regardless of the result code. This also eliminates the code duplication in my previous patch for creating the immediate event.
if the proxy_child fails and tevent_create_immediate(), too, proxy_reply() is not called.
Good catch. However, if we have ENOMEM here (the only way tevent_create_immediate() can fail) and there are events on the queue, then one of them may not fire. However, this is a hopefully impossible situation to get into, since we're freeing req just above it. I've added comments to that effect to the patch.
ACK
bye, Sumit
Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkzkM9kACgkQeiVVYja6o6Nu+wCfbHWGSi4s3Tl+b18LWQo5RrhH 2+8An2UY0I6aPWHr8Jgit39YCzL2a3K0 =lWnJ -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 11/17/2010 03:22 PM, Sumit Bose wrote:
On Wed, Nov 17, 2010 at 02:58:17PM -0500, Stephen Gallagher wrote: Good catch. However, if we have ENOMEM here (the only way tevent_create_immediate() can fail) and there are events on the queue, then one of them may not fire. However, this is a hopefully impossible situation to get into, since we're freeing req just above it. I've added comments to that effect to the patch.
ACK
Pushed to master.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
sssd-devel@lists.fedorahosted.org