From 03bdb26c5bf4e08d960e243d8a258e6f077a105e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Aug 2011 21:19:28 -0700 Subject: [PATCH] Move the alarm setup/teardown out of another_ldap_try() and into separate functions that bracket the another_ldap_try() loop. We now never leave a dangling alarm pending on success. --- source3/lib/smbldap.c | 91 ++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 802cb4821d2..5a1b0166cf7 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -1369,6 +1369,35 @@ static time_t calc_ldap_abs_endtime(int ldap_to) return time_mono(NULL)+ldap_to+1; } +static int end_ldap_local_alarm(time_t absolute_endtime, int rc) +{ + if (absolute_endtime) { + alarm(0); + CatchSignal(SIGALRM, SIG_IGN); + if (got_alarm) { + /* Client timeout error code. */ + got_alarm = 0; + return LDAP_TIMEOUT; + } + } + return rc; +} + +static void setup_ldap_local_alarm(struct smbldap_state *ldap_state, time_t absolute_endtime) +{ + time_t now = time_mono(NULL); + + if (absolute_endtime) { + got_alarm = 0; + CatchSignal(SIGALRM, gotalarm_sig); + alarm(absolute_endtime - now); + } + + if (ldap_state->pid != sys_getpid()) { + smbldap_close(ldap_state); + } +} + static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, int *attempts, time_t abs_endtime) { @@ -1384,17 +1413,6 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, goto no_next; } - if (*attempts == 0) { - if (abs_endtime) { - got_alarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm(abs_endtime - now); - } - - if (ldap_state->pid != sys_getpid()) - smbldap_close(ldap_state); - } - while (1) { if (*attempts != 0) @@ -1429,8 +1447,6 @@ static int another_ldap_try(struct smbldap_state *ldap_state, int *rc, } no_next: - alarm(0); - CatchSignal(SIGALRM, SIG_IGN); ldap_state->last_use = now; return False; } @@ -1451,7 +1467,6 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, time_t abs_endtime = calc_ldap_abs_endtime(to); struct timeval timeout; struct timeval *timeout_ptr = NULL; - int alarm_timer; size_t converted_size; SMB_ASSERT(ldap_state); @@ -1495,27 +1510,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, timeout_ptr = &timeout; } - /* Setup alarm timeout.... Do we need both of these ? JRA. - * Yes, I think we do need both of these. The server timeout only - * covers the case where the server's operation takes too long. It - * does not cover the case where the request hangs on its way to the - * server. The server side timeout is not strictly necessary, it's - * just a bit more kind to the server. VL. - * We prefer to get the server termination though because otherwise we - * have to rebind to the LDAP server aѕ we get a LDAP_SERVER_DOWN in the - * alarm termination case. So let's call the alarm 2s later, which - * should be enough for the server to report the timelimit exceeded. - * in case the ldal timeout is 0 (no limit) we set the _no_ alarm - * accordingly. BJ. */ - - got_alarm = 0; - CatchSignal(SIGALRM, gotalarm_sig); - alarm_timer = lp_ldap_timeout(); - if (alarm_timer > 0) { - alarm_timer += 2; - } - alarm(alarm_timer); - /* End setup timeout. */ + setup_ldap_local_alarm(ldap_state, abs_endtime); while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_search_ext_s(ldap_state->ldap_struct, base, scope, @@ -1546,15 +1541,7 @@ static int smbldap_search_ext(struct smbldap_state *ldap_state, } TALLOC_FREE(utf8_filter); - - /* Teardown timeout. */ - CatchSignal(SIGALRM, SIG_IGN); - alarm(0); - - if (got_alarm != 0) - return LDAP_TIMELIMIT_EXCEEDED; - - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_search(struct smbldap_state *ldap_state, @@ -1673,6 +1660,8 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_modify_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { @@ -1698,7 +1687,7 @@ int smbldap_modify(struct smbldap_state *ldap_state, const char *dn, LDAPMod *at } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs[]) @@ -1717,6 +1706,8 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_add_s(ldap_state->ldap_struct, utf8_dn, attrs); if (rc != LDAP_SUCCESS) { @@ -1742,7 +1733,7 @@ int smbldap_add(struct smbldap_state *ldap_state, const char *dn, LDAPMod *attrs } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) @@ -1761,6 +1752,8 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) return LDAP_NO_MEMORY; } + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_delete_s(ldap_state->ldap_struct, utf8_dn); if (rc != LDAP_SUCCESS) { @@ -1786,7 +1779,7 @@ int smbldap_delete(struct smbldap_state *ldap_state, const char *dn) } TALLOC_FREE(utf8_dn); - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } int smbldap_extended_operation(struct smbldap_state *ldap_state, @@ -1801,6 +1794,8 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, if (!ldap_state) return (-1); + setup_ldap_local_alarm(ldap_state, abs_endtime); + while (another_ldap_try(ldap_state, &rc, &attempts, abs_endtime)) { rc = ldap_extended_operation_s(ldap_state->ldap_struct, reqoid, reqdata, serverctrls, @@ -1827,7 +1822,7 @@ int smbldap_extended_operation(struct smbldap_state *ldap_state, } } - return rc; + return end_ldap_local_alarm(abs_endtime, rc); } /******************************************************************* -- 2.34.1