URL: https://github.com/SSSD/sssd/pull/327 Author: jhrozek Title: #327: RESPONDERS: Fix terminating idle connections Action: opened
PR body: """ The client_idle_handler() function tried to schedule another tevent timer to check for idle client connections in case the current connection was still valid, but in doing so, it also stored the current time into the last_request_time field of the client context. This kept the connection always alive, because the last_request_time could then never be older than the timeout. This patch changes the setup_client_idle_timer() function to only do what the synopsis says and set the idle timer. The caller (usually the function that accepts the connection) is supposed to store the request time itself. To test, revert the patch with the code fix and run the integration test in the other patch. """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/327/head:pr327 git checkout pr327
From d337f984ea11c20ea27bc93eb726439176d3da1b Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 11 Jul 2017 18:26:01 +0200 Subject: [PATCH 1/2] RESPONDERS: Fix terminating idle connections The client_idle_handler() function tried to schedule another tevent timer to check for idle client connections in case the current connection was still valid, but in doing so, it also stored the current time into the last_request_time field of the client context. This kept the connection always alive, because the last_request_time could then never be older than the timeout. This patch changes the setup_client_idle_timer() function to only do what the synopsis says and set the idle timer. The caller (usually the function that accepts the connection) is supposed to store the request time itself. Resolves: https://pagure.io/SSSD/sssd/issue/3448 --- src/responder/common/responder_common.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c index f81448e1f..9ac50cc07 100644 --- a/src/responder/common/responder_common.c +++ b/src/responder/common/responder_common.c @@ -608,7 +608,8 @@ static void accept_fd_handler(struct tevent_context *ev, cctx->ev = ev; cctx->rctx = rctx; - /* Set up the idle timer */ + /* Record the new time and set up the idle timer */ + reset_client_idle_timer(cctx); ret = setup_client_idle_timer(cctx); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -635,7 +636,7 @@ static void client_idle_handler(struct tevent_context *ev, if (cctx->last_request_time > now) { DEBUG(SSSDBG_IMPORTANT_INFO, "Time shift detected, re-scheduling the client timeout\n"); - goto end; + goto done; } if ((now - cctx->last_request_time) > cctx->rctx->client_idle_timeout) { @@ -649,7 +650,7 @@ static void client_idle_handler(struct tevent_context *ev, return; } -end: +done: setup_client_idle_timer(cctx); } @@ -662,11 +663,9 @@ errno_t reset_client_idle_timer(struct cli_ctx *cctx) static errno_t setup_client_idle_timer(struct cli_ctx *cctx) { - time_t now = time(NULL); struct timeval tv = tevent_timeval_current_ofs(cctx->rctx->client_idle_timeout/2, 0); - cctx->last_request_time = now; talloc_zfree(cctx->idle); cctx->idle = tevent_add_timer(cctx->ev, cctx, tv, client_idle_handler, cctx); From 7b02da170084c28e813313cd2c02f234c523f12c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 19 Jul 2017 14:22:17 +0200 Subject: [PATCH 2/2] TESTS: Integration test for idle timeout --- src/tests/intg/test_secrets.py | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py index f029e2787..3059c8b81 100644 --- a/src/tests/intg/test_secrets.py +++ b/src/tests/intg/test_secrets.py @@ -55,9 +55,9 @@ def create_sssd_secrets_fixture(request): assert secpid >= 0 if secpid == 0: - if subprocess.call([resp_path, "--uid=0", "--gid=0"]) != 0: - print("sssd_secrets failed to start") - sys.exit(99) + os.execv(resp_path, ("--uid=0", "--gid=0")) + print("sssd_secrets failed to start") + sys.exit(99) else: sock_path = os.path.join(config.RUNSTATEDIR, "secrets.socket") sck = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) @@ -100,11 +100,11 @@ def setup_for_secrets(request): [secrets] max_secrets = 10 max_payload_size = 2 + client_idle_timeout = 10 """).format(**locals()) create_conf_fixture(request, conf) - create_sssd_secrets_fixture(request) - return None + return create_sssd_secrets_fixture(request) def get_secrets_socket(): @@ -385,3 +385,32 @@ def test_containers(setup_for_secrets, secrets_cli): with pytest.raises(HTTPError) as err406: cli.create_container(container) assert str(err406.value).startswith("406") + + +def get_num_fds(pid): + procpath = os.path.join("/proc/", str(pid), "fd") + return len([fdname for fdname in os.listdir(procpath)]) + + +def test_idle_timeout(setup_for_secrets): + """ + Test that idle file descriptors are reaped after the idle timeout + passes + """ + if not curlwrap_tool: + pytest.skip("The tcurl tool is not available, skipping test") + + secpid = setup_for_secrets + sock_path = get_secrets_socket() + + nfds_pre = get_num_fds(secpid) + + sock = socket.socket(family=socket.AF_UNIX) + sock.connect(sock_path) + time.sleep(1) + nfds_conn = get_num_fds(secpid) + assert nfds_pre + 1 == nfds_conn + time.sleep(15) + + nfds_post = get_num_fds(secpid) + assert nfds_pre == nfds_post
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
