[Freeipa-devel] [PATCH 0020] Separate LDAP result from LDAP connection, fix deadlock.
Adam Tkac
atkac at redhat.com
Tue May 15 12:32:01 UTC 2012
On Mon, May 14, 2012 at 04:44:42PM +0200, Petr Spacek wrote:
> On 05/11/2012 12:26 PM, Adam Tkac wrote:
> >On Mon, May 07, 2012 at 02:49:07PM +0200, Petr Spacek wrote:
> >>Hello,
> >>
> >>this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/66:
> >>Plugin deadlocks during new zone load when connections == 1.
> >>
> >>It fixes structural problem, when LDAP query result was tied with
> >>LDAP connection up. It wasn't possible to release connection and
> >>work with query result after that.
> >>Described deadlock is consequence of this problematic design.
> >>
> >>Now LDAP connection is separated from LDAP result. Next planed patch
> >>will avoid "manual" connection management, so possibility of
> >>deadlock should be next to zero.
> >>
> >>Petr^2 Spacek
> >
> >Hello Peter,
> >
> >good work, please check my comments below.
> >
> >Regards, Adam
> >
> >> From 8ee1fd607531ef71369e99c9228456baea45b65d Mon Sep 17 00:00:00 2001
> >>From: Petr Spacek<pspacek at redhat.com>
> >>Date: Mon, 7 May 2012 12:51:09 +0200
> >>Subject: [PATCH] Separate LDAP result from LDAP connection, fix deadlock.
> >> https://fedorahosted.org/bind-dyndb-ldap/ticket/66
> >> Signed-off-by: Petr Spacek<pspacek at redhat.com>
>
> Hello Adam,
>
> thanks for ideas/improvements!
>
> Reworked patch is attached. I did all proposed changes except this one:
>
> @ ldap_psearch_watcher:
> >> restart:
> (... snip ...)
> >> soft_err:
> >>-
> >>- ldap_msgfree(conn->result);
> >>- ldap_entrylist_destroy(conn->mctx,
> >>- &conn->ldap_entries);
> >>+ ;
> >
> >Empty label "soft_err:" is useless, please remove it and use "continue;" on
> >appropriate places;
>
> I think "continue" in this place can lead to memory leak, so I
> removed soft_err by other way.
Thanks for the patch, now it looks fine to me, except that it doesn't apply on
the current master:
[atkac at drtic bind-dyndb-ldap]$ git am ../bind-dyndb-ldap-pspacek-0020-2-Separate-LDAP-result-from-LDAP-connection-fix-deadlo.patch
Applying: Separate LDAP result from LDAP connection, fix deadlock. https://fedorahosted.org/bind-dyndb-ldap/ticket/66 Signed-off-by: Petr Spacek <pspacek at redhat.com>
error: patch failed: src/ldap_helper.c:271
error: src/ldap_helper.c: patch does not apply
Patch failed at 0001 Separate LDAP result from LDAP connection, fix deadlock. https://fedorahosted.org/bind-dyndb-ldap/ticket/66 Signed-off-by: Petr Spacek <pspacek at redhat.com>
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Please rebase the patch and then push it, you don't have to resend it here.
Regards, Adam
> From 4c8c8c1ec7777217d3219a0380f6baec4b41248d Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Mon, 7 May 2012 12:51:09 +0200
> Subject: [PATCH] Separate LDAP result from LDAP connection, fix deadlock.
> https://fedorahosted.org/bind-dyndb-ldap/ticket/66
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
>
> ---
> src/ldap_helper.c | 269 ++++++++++++++++++++++++++++++++---------------------
> 1 files changed, 162 insertions(+), 107 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 304c37296f8f3a428c4c72b45fe4154b2c9e954c..dae3e28804b602ca91d813c4d68d258e7065608f 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -108,6 +108,7 @@
> * must acquire the semaphore and the lock.
> */
>
> +typedef struct ldap_qresult ldap_qresult_t;
> typedef struct ldap_connection ldap_connection_t;
> typedef struct ldap_pool ldap_pool_t;
> typedef struct ldap_auth_pair ldap_auth_pair_t;
> @@ -188,28 +189,28 @@ struct ldap_connection {
> ld_string_t *query_string;
>
> LDAP *handle;
> - LDAPMessage *result;
> LDAPControl *serverctrls[2]; /* psearch/NULL or NULL/NULL */
> int msgid;
>
> /* Parsing. */
> isc_lex_t *lex;
> isc_buffer_t rdata_target;
> unsigned char *rdata_target_mem;
>
> - /* Cache. */
> - ldap_entrylist_t ldap_entries;
> -
> /* For reconnection logic. */
> isc_time_t next_reconnect;
> unsigned int tries;
> +};
>
> - /* Temporary stuff. */
> - LDAPMessage *entry;
> - BerElement *ber;
> - char *attribute;
> - char **values;
> - char *dn;
> +/**
> + * Result from single LDAP query.
> + */
> +struct ldap_qresult {
> + isc_mem_t *mctx;
> + LDAPMessage *result;
> +
> + /* Cache. */
> + ldap_entrylist_t ldap_entries;
> };
>
> /*
> @@ -271,9 +272,10 @@ static int handle_connection_error(ldap_instance_t *ldap_inst,
> ldap_connection_t *ldap_conn, isc_boolean_t force,
> isc_result_t *result);
> static isc_result_t ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> - const char *base,
> - int scope, char **attrs, int attrsonly, const char *filter, ...);
> -static void ldap_query_free(ldap_connection_t *ldap_conn);
> + ldap_qresult_t **ldap_qresultp, const char *base, int scope, char **attrs,
> + int attrsonly, const char *filter, ...);
> +static isc_result_t ldap_query_create(isc_mem_t *mctx, ldap_qresult_t **ldap_qresultp);
> +static void ldap_query_free(isc_boolean_t prepare_reuse, ldap_qresult_t **ldap_qresultp);
>
> /* Functions for writing to LDAP. */
> static isc_result_t ldap_modify_do(ldap_connection_t *ldap_conn, const char *dn,
> @@ -1095,6 +1097,8 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst)
> {
> isc_result_t result = ISC_R_SUCCESS;
> ldap_connection_t *ldap_conn = NULL;
> + ldap_qresult_t *ldap_config_qresult = NULL;
> + ldap_qresult_t *ldap_zones_qresult = NULL;
> int zone_count = 0;
> ldap_entry_t *entry;
> dns_rbt_t *rbt = NULL;
> @@ -1119,31 +1123,31 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst)
>
> log_debug(2, "refreshing list of zones for %s", ldap_inst->db_name);
>
> + /* Query for configuration and zones from LDAP and release LDAP connection
> + * before processing them. It prevents deadlock in situations where
> + * ldap_parse_zoneentry() requests another connection. */
> CHECK(ldap_pool_getconnection(ldap_inst->pool, &ldap_conn));
> -
> - CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(ldap_inst->base),
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_config_qresult, str_buf(ldap_inst->base),
> LDAP_SCOPE_SUBTREE, config_attrs, 0,
> "(objectClass=idnsConfigObject)"));
> -
> - for (entry = HEAD(ldap_conn->ldap_entries);
> - entry != NULL;
> - entry = NEXT(entry, link)) {
> - CHECK(ldap_parse_configentry(entry, ldap_inst));
> - }
> -
> - ldap_query_free(ldap_conn);
> -
> - CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(ldap_inst->base),
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_zones_qresult, str_buf(ldap_inst->base),
> LDAP_SCOPE_SUBTREE, zone_attrs, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
> + ldap_pool_putconnection(ldap_inst->pool, &ldap_conn);
> +
> + for (entry = HEAD(ldap_config_qresult->ldap_entries);
> + entry != NULL;
> + entry = NEXT(entry, link)) {
> + CHECK(ldap_parse_configentry(entry, ldap_inst));
> + }
>
> /*
> * Create RB-tree with all zones stored in LDAP for cross check
> * with registered zones in plugin.
> */
> CHECK(dns_rbt_create(ldap_inst->mctx, NULL, NULL, &rbt));
>
> - for (entry = HEAD(ldap_conn->ldap_entries);
> + for (entry = HEAD(ldap_zones_qresult->ldap_entries);
> entry != NULL;
> entry = NEXT(entry, link)) {
>
> @@ -1232,6 +1236,8 @@ cleanup:
> if (invalidate_nodechain)
> dns_rbtnodechain_invalidate(&chain);
>
> + ldap_query_free(ISC_FALSE, &ldap_config_qresult);
> + ldap_query_free(ISC_FALSE, &ldap_zones_qresult);
> ldap_pool_putconnection(ldap_inst->pool, &ldap_conn);
>
> log_debug(2, "finished refreshing list of zones");
> @@ -1387,6 +1393,7 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *nam
> {
> isc_result_t result;
> ldap_connection_t *ldap_conn = NULL;
> + ldap_qresult_t *ldap_qresult = NULL;
> ldap_entry_t *entry;
> ld_string_t *string = NULL;
> ldapdb_node_t *node;
> @@ -1403,15 +1410,15 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *nam
> CHECK(str_new(mctx, &string));
> CHECK(dnsname_to_dn(ldap_inst->zone_register, name, string));
>
> - CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(string),
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_qresult, str_buf(string),
> LDAP_SCOPE_SUBTREE, NULL, 0, "(objectClass=idnsRecord)"));
>
> - if (EMPTY(ldap_conn->ldap_entries)) {
> + if (EMPTY(ldap_qresult->ldap_entries)) {
> result = ISC_R_NOTFOUND;
> goto cleanup;
> }
>
> - for (entry = HEAD(ldap_conn->ldap_entries);
> + for (entry = HEAD(ldap_qresult->ldap_entries);
> entry != NULL;
> entry = NEXT(entry, link)) {
> node = NULL;
> @@ -1444,6 +1451,7 @@ ldapdb_nodelist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *nam
> result = ISC_R_SUCCESS;
>
> cleanup:
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(ldap_inst->pool, &ldap_conn);
> str_destroy(&string);
>
> @@ -1456,6 +1464,7 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *na
> {
> isc_result_t result;
> ldap_connection_t *ldap_conn = NULL;
> + ldap_qresult_t *ldap_qresult = NULL;
> ldap_entry_t *entry;
> ld_string_t *string = NULL;
> ldap_cache_t *cache;
> @@ -1478,15 +1487,15 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *na
> CHECK(dnsname_to_dn(ldap_inst->zone_register, name, string));
>
> CHECK(ldap_pool_getconnection(ldap_inst->pool, &ldap_conn));
> - CHECK(ldap_query(ldap_inst, ldap_conn, str_buf(string),
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_qresult, str_buf(string),
> LDAP_SCOPE_BASE, NULL, 0, "(objectClass=idnsRecord)"));
>
> - if (EMPTY(ldap_conn->ldap_entries)) {
> + if (EMPTY(ldap_qresult->ldap_entries)) {
> result = ISC_R_NOTFOUND;
> goto cleanup;
> }
>
> - for (entry = HEAD(ldap_conn->ldap_entries);
> + for (entry = HEAD(ldap_qresult->ldap_entries);
> entry != NULL;
> entry = NEXT(entry, link)) {
> if (ldap_parse_rrentry(mctx, entry, ldap_conn,
> @@ -1504,6 +1513,7 @@ ldapdb_rdatalist_get(isc_mem_t *mctx, ldap_instance_t *ldap_inst, dns_name_t *na
> result = ISC_R_NOTFOUND;
>
> cleanup:
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(ldap_inst->pool, &ldap_conn);
> str_destroy(&string);
>
> @@ -1601,16 +1611,24 @@ cleanup:
> return result;
> }
>
> +/**
> + * @param ldap_conn A LDAP connection structure obtained via ldap_get_connection().
> + * @param ldap_qresult New ldap_qresult structure will be allocated and pointer
> + * to it will be returned through this parameter. The result
> + * has to be freed by caller via ldap_query_free().
> + */
> static isc_result_t
> ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> - const char *base, int scope, char **attrs,
> + ldap_qresult_t **ldap_qresultp, const char *base, int scope, char **attrs,
> int attrsonly, const char *filter, ...)
> {
> va_list ap;
> isc_result_t result;
> + ldap_qresult_t *ldap_qresult = NULL;
> int cnt;
>
> REQUIRE(ldap_conn != NULL);
> + REQUIRE(ldap_qresultp != NULL && *ldap_qresultp == NULL);
>
> va_start(ap, filter);
> str_vsprintf(ldap_conn->query_string, filter, ap);
> @@ -1629,62 +1647,96 @@ ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
> return result;
> }
>
> + CHECK(ldap_query_create(ldap_inst->mctx, &ldap_qresult));
> +
> do {
> int ret;
>
> ret = ldap_search_ext_s(ldap_conn->handle, base, scope,
> str_buf(ldap_conn->query_string),
> attrs, attrsonly, NULL, NULL, NULL,
> - LDAP_NO_LIMIT, &ldap_conn->result);
> + LDAP_NO_LIMIT, &(ldap_qresult->result));
> if (ret == 0) {
> ldap_conn->tries = 0;
> - cnt = ldap_count_entries(ldap_conn->handle, ldap_conn->result);
> + cnt = ldap_count_entries(ldap_conn->handle, ldap_qresult->result);
> log_debug(2, "entry count: %d", cnt);
>
> result = ldap_entrylist_create(ldap_conn->mctx,
> ldap_conn->handle,
> - ldap_conn->result,
> - &ldap_conn->ldap_entries);
> + ldap_qresult->result,
> + &ldap_qresult->ldap_entries);
> if (result != ISC_R_SUCCESS) {
> log_error("failed to save LDAP query results");
> - return result;
> }
>
> - return ISC_R_SUCCESS;
> + break;
> }
> + /* Special case is LDAP_NO_SUCH_OBJECT: handle_connection_error() will
> + * set result = ISC_R_SUCCESS and break the cycle. */
> } while (handle_connection_error(ldap_inst, ldap_conn, ISC_FALSE, &result));
>
> +cleanup:
> + if (result == ISC_R_SUCCESS)
> + *ldap_qresultp = ldap_qresult;
> + else
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> return result;
> }
>
> +/**
> + * Allocate and initialize new ldap_qresult structure.
> + * @param[out] ldap_qresultp Newly allocated ldap_qresult structure.
> + * @return ISC_R_SUCCESS or ISC_R_NOMEMORY (from CHECKED_MEM_GET_PTR)
> + */
> +static isc_result_t
> +ldap_query_create(isc_mem_t *mctx, ldap_qresult_t **ldap_qresultp) {
> + ldap_qresult_t *ldap_qresult = NULL;
> + isc_result_t result;
> +
> + CHECKED_MEM_GET_PTR(mctx, ldap_qresult);
> + ldap_qresult->mctx = mctx;
> + ldap_qresult->result = NULL;
> + INIT_LIST(ldap_qresult->ldap_entries);
> +
> + *ldap_qresultp = ldap_qresult;
> + return ISC_R_SUCCESS;
> +
> +cleanup:
> + return result;
> +}
> +
> +/**
> + * Free LDAP query result. Can free whole structure or internal parts only.
> + * Freeing internal parts is suitable before reusing the structure.
> + * @param[in] prepare_reuse ISC_TRUE implies freeing internal parts,
> + * but not the whole structure.
> + * @param[in,out] ldap_qresultp Pointer to freed query. Will be set to NULL
> + * if prepare_reuse == ISC_FALSE.
> + */
> static void
> -ldap_query_free(ldap_connection_t *ldap_conn)
> +ldap_query_free(isc_boolean_t prepare_reuse, ldap_qresult_t **ldap_qresultp)
> {
> - if (ldap_conn == NULL)
> + ldap_qresult_t *qresult;
> + REQUIRE(ldap_qresultp != NULL);
> +
> + qresult = *ldap_qresultp;
> +
> + if (qresult == NULL)
> return;
>
> - if (ldap_conn->dn) {
> - ldap_memfree(ldap_conn->dn);
> - ldap_conn->dn = NULL;
> - }
> - if (ldap_conn->values) {
> - ldap_value_free(ldap_conn->values);
> - ldap_conn->values = NULL;
> - }
> - if (ldap_conn->attribute) {
> - ldap_memfree(ldap_conn->attribute);
> - ldap_conn->attribute = NULL;
> - }
> - if (ldap_conn->ber) {
> - ber_free(ldap_conn->ber, 0);
> - ldap_conn->ber = NULL;
> - }
> - if (ldap_conn->result) {
> - ldap_msgfree(ldap_conn->result);
> - ldap_conn->result = NULL;
> + if (qresult->result) {
> + ldap_msgfree(qresult->result);
> + qresult->result = NULL;
> }
>
> - ldap_entrylist_destroy(ldap_conn->mctx, &ldap_conn->ldap_entries);
> + ldap_entrylist_destroy(qresult->mctx, &qresult->ldap_entries);
> +
> + if (prepare_reuse) {
> + INIT_LIST(qresult->ldap_entries);
> + } else { /* free whole structure */
> + SAFE_MEM_PUT_PTR(qresult->mctx, qresult);
> + *ldap_qresultp = NULL;
> + }
> }
>
> /* FIXME: Tested with SASL/GSSAPI/KRB5 only */
> @@ -2229,6 +2281,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
> isc_result_t result;
> isc_mem_t *mctx = ldap_inst->mctx;
> ldap_connection_t *ldap_conn = NULL;
> + ldap_qresult_t *ldap_qresult = NULL;
> ld_string_t *owner_dn = NULL;
> LDAPMod *change[3] = { NULL };
> LDAPMod *change_ptr = NULL;
> @@ -2255,12 +2308,12 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
> }
>
> CHECK(ldap_pool_getconnection(ldap_inst->pool, &ldap_conn));
> - CHECK(ldap_query(ldap_inst, ldap_conn, zone_dn,
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_qresult, zone_dn,
> LDAP_SCOPE_BASE, attrs, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
>
> /* only 0 or 1 active zone can be returned from query */
> - entry = HEAD(ldap_conn->ldap_entries);
> + entry = HEAD(ldap_qresult->ldap_entries);
> if (entry == NULL) {
> log_debug(3, "Active zone %s not found", zone_dn);
> result = ISC_R_NOTFOUND;
> @@ -2392,14 +2445,14 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
> char *owner_zone_dn_ptr = strstr(str_buf(owner_dn_ptr),", ") + 1;
>
> /* Get attribute "idnsAllowDynUpdate" for reverse zone or use default. */
> - ldap_query_free(ldap_conn);
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> zone_dyn_update = ldap_inst->dyn_update;
> - CHECK(ldap_query(ldap_inst, ldap_conn, owner_zone_dn_ptr,
> + CHECK(ldap_query(ldap_inst, ldap_conn, &ldap_qresult, owner_zone_dn_ptr,
> LDAP_SCOPE_BASE, attrs, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
>
> /* Only 0 or 1 active zone can be returned from query. */
> - entry = HEAD(ldap_conn->ldap_entries);
> + entry = HEAD(ldap_qresult->ldap_entries);
> if (entry == NULL) {
> log_debug(3, "Active zone %s not found", zone_dn);
> result = ISC_R_NOTFOUND;
> @@ -2485,6 +2538,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
> }
>
> cleanup:
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(ldap_inst->pool, &ldap_conn);
> str_destroy(&owner_dn_ptr);
> str_destroy(&owner_dn);
> @@ -2587,7 +2641,6 @@ ldap_pool_getconnection(ldap_pool_t *pool, ldap_connection_t ** conn)
>
> RUNTIME_CHECK(ldap_conn != NULL);
>
> - INIT_LIST(ldap_conn->ldap_entries);
> *conn = ldap_conn;
>
> cleanup:
> @@ -2607,7 +2660,6 @@ ldap_pool_putconnection(ldap_pool_t *pool, ldap_connection_t **conn)
> if (ldap_conn == NULL)
> return;
>
> - ldap_query_free(ldap_conn);
> UNLOCK(&ldap_conn->lock);
> semaphore_signal(&pool->conn_semaphore);
>
> @@ -2739,6 +2791,7 @@ update_action(isc_task_t *task, isc_event_t *event)
> isc_result_t result ;
> ldap_instance_t *inst = NULL;
> ldap_connection_t *conn = NULL;
> + ldap_qresult_t *ldap_qresult = NULL;
> ldap_entry_t *entry;
> isc_boolean_t delete = ISC_TRUE;
> isc_mem_t *mctx;
> @@ -2757,11 +2810,11 @@ update_action(isc_task_t *task, isc_event_t *event)
>
> CHECK(ldap_pool_getconnection(inst->pool, &conn));
>
> - CHECK(ldap_query(inst, conn, pevent->dn,
> + CHECK(ldap_query(inst, conn, &ldap_qresult, pevent->dn,
> LDAP_SCOPE_BASE, attrs, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
>
> - for (entry = HEAD(conn->ldap_entries);
> + for (entry = HEAD(ldap_qresult->ldap_entries);
> entry != NULL;
> entry = NEXT(entry, link)) {
> delete = ISC_FALSE;
> @@ -2779,6 +2832,7 @@ cleanup:
> "Zones can be outdated, run `rndc reload`",
> pevent->dn);
>
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(inst->pool, &conn);
> isc_mem_free(mctx, pevent->dbname);
> isc_mem_free(mctx, pevent->dn);
> @@ -2793,6 +2847,7 @@ update_config(isc_task_t *task, isc_event_t *event)
> isc_result_t result ;
> ldap_instance_t *inst = NULL;
> ldap_connection_t *conn = NULL;
> + ldap_qresult_t *ldap_qresult = NULL;
> ldap_entry_t *entry;
> isc_mem_t *mctx;
> char *attrs[] = {
> @@ -2810,14 +2865,14 @@ update_config(isc_task_t *task, isc_event_t *event)
>
> CHECK(ldap_pool_getconnection(inst->pool, &conn));
>
> - CHECK(ldap_query(inst, conn, pevent->dn,
> + CHECK(ldap_query(inst, conn, &ldap_qresult, pevent->dn,
> LDAP_SCOPE_BASE, attrs, 0,
> "(objectClass=idnsConfigObject)"));
>
> - if (EMPTY(conn->ldap_entries))
> - log_error("Config object can not be empty");
> + if (EMPTY(ldap_qresult->ldap_entries))
> + log_error("Config object can not be empty"); // WHY?
>
> - for (entry = HEAD(conn->ldap_entries);
> + for (entry = HEAD(ldap_qresult->ldap_entries);
> entry != NULL;
> entry = NEXT(entry, link)) {
> result = ldap_parse_configentry(entry, inst);
> @@ -2832,6 +2887,7 @@ cleanup:
> "Configuration can be outdated, run `rndc reload`",
> pevent->dn);
>
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(inst->pool, &conn);
> isc_mem_free(mctx, pevent->dbname);
> isc_mem_free(mctx, pevent->dn);
> @@ -3071,6 +3127,7 @@ ldap_psearch_watcher(isc_threadarg_t arg)
> {
> ldap_instance_t *inst = (ldap_instance_t *)arg;
> ldap_connection_t *conn;
> + ldap_qresult_t *ldap_qresult = NULL;
> struct timeval tv;
> int ret, cnt;
> isc_result_t result;
> @@ -3108,6 +3165,8 @@ ldap_psearch_watcher(isc_threadarg_t arg)
> ldap_connect(inst, conn, ISC_TRUE);
> }
>
> + CHECK(ldap_query_create(conn->mctx, &ldap_qresult));
> +
> restart:
> /* Perform initial lookup */
> if (inst->psearch) {
> @@ -3134,7 +3193,7 @@ restart:
>
> while (!inst->exiting) {
> ret = ldap_result(conn->handle, conn->msgid, 0, &tv,
> - &conn->result);
> + &ldap_qresult->result);
>
> if (ret <= 0) {
> int ok;
> @@ -3153,58 +3212,54 @@ restart:
> case LDAP_RES_SEARCH_ENTRY:
> break;
> default:
> - log_debug(3, "Ignoring psearch msg with retcode %x",
> - ret);
> + log_debug(3, "Ignoring psearch msg with retcode %x", ret);
> + break;
> }
>
> conn->tries = 0;
> - cnt = ldap_count_entries(conn->handle, conn->result);
> + cnt = ldap_count_entries(conn->handle, ldap_qresult->result);
>
> if (cnt > 0) {
> log_debug(3, "Got psearch updates (%d)", cnt);
> - result = ldap_entrylist_append(conn->mctx,
> + result = ldap_entrylist_append(ldap_qresult->mctx,
> conn->handle,
> - conn->result,
> - &conn->ldap_entries);
> - if (result != ISC_R_SUCCESS) {
> + ldap_qresult->result,
> + &ldap_qresult->ldap_entries);
> +
> + if (result == ISC_R_SUCCESS) {
> + ldap_entry_t *entry;
> + for (entry = HEAD(ldap_qresult->ldap_entries);
> + entry != NULL;
> + entry = NEXT(entry, link)) {
> + LDAPControl **ctrls = NULL;
> + ret = ldap_get_entry_controls(conn->handle,
> + entry->ldap_entry,
> + &ctrls);
> + if (ret != LDAP_SUCCESS) {
> + log_error("failed to extract controls "
> + "from psearch update. Zones "
> + "might be outdated, run "
> + "`rndc reload`");
> + break;
> + }
> + psearch_update(inst, entry, ctrls);
> + }
> + } else { /* result != ISC_R_SUCCESS */
> /*
> * Error means inconsistency of our zones
> * data.
> */
> log_error("ldap_psearch_watcher failed, zones "
> "might be outdated. Run `rndc reload`");
> - goto soft_err;
> }
> -
> - ldap_entry_t *entry;
> - for (entry = HEAD(conn->ldap_entries);
> - entry != NULL;
> - entry = NEXT(entry, link)) {
> - LDAPControl **ctrls = NULL;
> - ret = ldap_get_entry_controls(conn->handle,
> - entry->ldap_entry,
> - &ctrls);
> - if (ret != LDAP_SUCCESS) {
> - log_error("failed to extract controls "
> - "from psearch update. Zones "
> - "might be outdated, run "
> - "`rndc reload");
> - goto soft_err;
> - }
> -
> - psearch_update(inst, entry, ctrls);
> - }
> -soft_err:
> -
> - ldap_msgfree(conn->result);
> - ldap_entrylist_destroy(conn->mctx,
> - &conn->ldap_entries);
> }
> + ldap_query_free(ISC_TRUE, &ldap_qresult);
> }
>
> log_debug(1, "Ending ldap_psearch_watcher");
>
> cleanup:
> + ldap_query_free(ISC_FALSE, &ldap_qresult);
> ldap_pool_putconnection(inst->pool, &conn);
>
> return (isc_threadresult_t)0;
> --
> 1.7.7.6
>
--
Adam Tkac, Red Hat, Inc.
More information about the Freeipa-devel
mailing list