Thursday, May 15, 2014

[389-commits] Branch '389-ds-base-1.3.1' - ldap/servers

ldap/servers/slapd/back-ldbm/ancestorid.c | 28 ++--
ldap/servers/slapd/back-ldbm/filterindex.c | 50 ++++---
ldap/servers/slapd/back-ldbm/idl.c | 92 ++++++-------
ldap/servers/slapd/back-ldbm/idl_common.c | 12 -
ldap/servers/slapd/back-ldbm/idl_new.c | 20 +--
ldap/servers/slapd/back-ldbm/import-merge.c | 6
ldap/servers/slapd/back-ldbm/import-threads.c | 4
ldap/servers/slapd/back-ldbm/index.c | 16 +-
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 4
ldap/servers/slapd/back-ldbm/ldbm_search.c | 166 ++++++++++++++-----------
ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 23 +--
ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2
ldap/servers/slapd/back-ldbm/seq.c | 4
ldap/servers/slapd/back-ldbm/vlv.c | 2
15 files changed, 231 insertions(+), 200 deletions(-)

New commits:
commit ff4c7a7b3f65589f93d88aadfcbe95976a9b31a9
Author: Noriko Hosoi <nhosoi@redhat.com>
Date: Thu May 15 14:43:28 2014 -0700

Ticket #47780 - Some VLV search request causes memory leaks

Fix description:
. Modified idl_free interface as follows so that passed idl is cleared
with NULL once the IDList is successfully freed.
-idl_free(IDList *idl)
+idl_free(IDList **idl)
This change is used to clean up search candidates when ldbm_back_
search_cleanup (ldbm_search.c) is called as an error return. The
cleanup function frees the search candidates when it's not NULL and
it's not assigned to sr_candidates field in the search result. This
fixes a memory leak when VLV/Sort op fails.
. ldbm_back_search_cleanup (ldbm_search.c) calls slapi_send_ldap_result
if an ldap error is passed to the function. The logic used to be
"if (ldap_result>=LDAP_SUCCESS)", which is based upon that mozldap
return codes are all positive. Supporting openldap library, there
is a chance to get a negative return code (e.g. LDAP_PARAM_ERROR ==
-9). This patch supports the negative return codes, as well.
. In ldbm_back_search (ldbm_search.c) vlv_filter_candidates could
ruturn errors such as and LDAP_TIMELIMIT_EXCEEDED, LDAP_ADMINLIMIT_
EXCEEDED. The search results are supposed to be returned to the
client with the error code if the control is not critical. The code
is added.
. The VLV operation stores the result in vlv_response_control.result
in ldbm_back_search (ldbm_search.c), which occurs at 3 places, vlv_
filter_candidates, sort_candidates and vlv_trim_candidates_txn.
The return code from the latter calls used to override the former
return code. This patch fixes it to respect the former return code.

https://fedorahosted.org/389/ticket/47780

Reviewed by rmeggins@redhat.com (Thank you, Rich!!)

(cherry picked from commit 1118cc9b61a60c704250080d6af8785e4f8d28af)
(cherry picked from commit e9f86dab95fd600b414cf1c4cf6f7c733348b758)

diff --git a/ldap/servers/slapd/back-ldbm/ancestorid.c b/ldap/servers/slapd/back-ldbm/ancestorid.c
index 8722b68..54b08ad 100644
--- a/ldap/servers/slapd/back-ldbm/ancestorid.c
+++ b/ldap/servers/slapd/back-ldbm/ancestorid.c
@@ -136,7 +136,7 @@ static int ldbm_get_nonleaf_ids(backend *be, DB_TXN *txn, IDList **idl)
LDAPDebug(LDAP_DEBUG_TRACE, "found %lu nodes for ancestorid\n",
(u_long)IDL_NIDS(nodes), 0, 0);
} else {
- idl_free(nodes);
+ idl_free(&nodes);
*idl = NULL;
}

@@ -264,7 +264,7 @@ static int ldbm_ancestorid_default_create_index(backend *be)
/* Insert into ancestorid for this node */
if (id2idl_hash_lookup(ht, &id, &ididl)) {
descendants = idl_union_allids(be, ai_aid, ididl->idl, children);
- idl_free(children);
+ idl_free(&children);
if (id2idl_hash_remove(ht, &id) == 0) {
LDAPDebug(LDAP_DEBUG_ANY, "ancestorid hash_remove failed\n", 0,0,0);
} else {
@@ -279,21 +279,21 @@ static int ldbm_ancestorid_default_create_index(backend *be)
/* Get parentid for this entry */
ret = ldbm_parentid(be, txn, id, &parentid);
if (ret != 0) {
- idl_free(descendants);
+ idl_free(&descendants);
break;
}

/* A suffix entry does not have a parent */
if (parentid == NOID) {
- idl_free(descendants);
+ idl_free(&descendants);
continue;
}

/* Insert into ancestorid for this node's parent */
if (id2idl_hash_lookup(ht, &parentid, &ididl)) {
IDList *idl = idl_union_allids(be, ai_aid, ididl->idl, descendants);
- idl_free(descendants);
- idl_free(ididl->idl);
+ idl_free(&descendants);
+ idl_free(&(ididl->idl));
ididl->idl = idl;
} else {
ididl = (id2idl*)slapi_ch_calloc(1,sizeof(id2idl));
@@ -324,7 +324,7 @@ static int ldbm_ancestorid_default_create_index(backend *be)
id2idl_hash_destroy(ht);

/* Free any leftover idlists */
- idl_free(nodes);
+ idl_free(&nodes);

/* Release the parentid file */
if (db_pid != NULL) {
@@ -456,7 +456,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be)
/* Insert into ancestorid for this node */
ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid);
if (ret != 0) {
- idl_free(children);
+ idl_free(&children);
break;
}

@@ -467,13 +467,13 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be)
slapi_log_error(SLAPI_LOG_FATAL, sourcefile,
"Error: ldbm_parentid on node index [" ID_FMT "] of [" ID_FMT "]\n",
nids, nodes->b_nids);
- idl_free(children);
+ idl_free(&children);
goto out;
}

/* A suffix entry does not have a parent */
if (parentid == NOID) {
- idl_free(children);
+ idl_free(&children);
break;
}

@@ -485,7 +485,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be)
/* Insert into ancestorid for this node's parent */
ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid);
if (ret != 0) {
- idl_free(children);
+ idl_free(&children);
goto out;
}
id = parentid;
@@ -504,7 +504,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be)
}

/* Free any leftover idlists */
- idl_free(nodes);
+ idl_free(&nodes);

/* Release the parentid file */
if (db_pid != NULL) {
@@ -583,7 +583,7 @@ static int ldbm_parentid(backend *be, DB_TXN *txn, ID id, ID *ppid)

static void id2idl_free(id2idl **ididl)
{
- idl_free((*ididl)->idl);
+ idl_free(&((*ididl)->idl));
slapi_ch_free((void**)ididl);
}

@@ -785,7 +785,7 @@ static int ldbm_ancestorid_index_update(
break;
}
node_id = idl_firstid(idl);
- idl_free(idl);
+ idl_free(&idl);
}

/* Update ancestorid for the base entry */
diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c
index d1fddf9..8a52d0d 100644
--- a/ldap/servers/slapd/back-ldbm/filterindex.c
+++ b/ldap/servers/slapd/back-ldbm/filterindex.c
@@ -399,7 +399,7 @@ presence_candidates(
LDAPDebug(LDAP_DEBUG_TRACE,
"fallback to eq index as pres index gave allids\n",
0, 0, 0);
- idl_free(idl);
+ idl_free(&idl);
idl = index_range_read_ext(pb, be, type, indextype_EQUALITY,
SLAPI_OP_GREATER_OR_EQUAL,
NULL, NULL, 0, &txn, err, allidslimit);
@@ -481,7 +481,7 @@ extensible_candidates(
else if (keys == NULL || keys[0] == NULL)
{
/* no keys */
- idl_free (idl);
+ idl_free (&idl);
idl = idl_allids (be);
}
else
@@ -516,8 +516,8 @@ extensible_candidates(
else
{
IDList* tmp = idl_intersection (be, idl2, idl3);
- idl_free (idl2);
- idl_free (idl3);
+ idl_free (&idl2);
+ idl_free (&idl3);
idl2 = tmp;
}
if (idl2 == NULL) break; /* look no further */
@@ -529,8 +529,8 @@ extensible_candidates(
else if (idl2 != NULL)
{
IDList* tmp = idl_union (be, idl, idl2);
- idl_free (idl);
- idl_free (idl2);
+ idl_free (&idl);
+ idl_free (&idl2);
idl = tmp;
}
}
@@ -797,7 +797,7 @@ list_candidates(
{
LDAPDebug( LDAP_DEBUG_TRACE,
"<= list_candidates NULL\n", 0, 0, 0 );
- idl_free( idl );
+ idl_free( &idl );
idl = NULL;
goto out;
}
@@ -807,7 +807,7 @@ list_candidates(
== NULL && ftype == LDAP_FILTER_AND ) {
LDAPDebug( LDAP_DEBUG_TRACE,
"<= list_candidates NULL\n", 0, 0, 0 );
- idl_free( idl );
+ idl_free( &idl );
idl = NULL;
goto out;
}
@@ -822,18 +822,24 @@ list_candidates(
}
} else if ( ftype == LDAP_FILTER_AND ) {
if (isnot) {
- IDList *new_idl = NULL;
- int notin_result = 0;
- notin_result = idl_notin( be, idl, tmp, &new_idl );
- if (notin_result) {
- idl_free(idl);
- idl = new_idl;
+ /*
+ * If tmp is NULL or ALLID, idl_notin just duplicates idl.
+ * We don't have to do it.
+ */
+ if (!tmp && !idl_is_allids(tmp)) {
+ IDList *new_idl = NULL;
+ int notin_result = 0;
+ notin_result = idl_notin( be, idl, tmp, &new_idl );
+ if (notin_result) {
+ idl_free(&idl);
+ idl = new_idl;
+ }
}
} else {
idl = idl_intersection(be, idl, tmp);
- idl_free( tmp2 );
+ idl_free( &tmp2 );
}
- idl_free( tmp );
+ idl_free( &tmp );
/* stop if the list has gotten too small */
if ((idl == NULL) ||
(idl_length(idl) <= FILTER_TEST_THRESHOLD))
@@ -843,8 +849,8 @@ list_candidates(
slapi_pblock_get( pb, SLAPI_OPERATION, &operation );

idl = idl_union( be, idl, tmp );
- idl_free( tmp );
- idl_free( tmp2 );
+ idl_free( &tmp );
+ idl_free( &tmp2 );
/* stop if we're already committed to an exhaustive
* search. :(
*/
@@ -853,7 +859,7 @@ list_candidates(
if (op_is_pagedresults(operation)) {
int nids = IDL_NIDS(idl);
if ( allidslimit > 0 && nids > allidslimit ) {
- idl_free( idl );
+ idl_free( &idl );
idl = idl_allids( be );
}
}
@@ -976,7 +982,7 @@ keys2idl(
}

No comments:

Post a Comment