Skip to content

Commit

Permalink
Ticket #47780 - Some VLV search request causes memory leaks
Browse files Browse the repository at this point in the history 
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 1118cc9)
  • Loading branch information
@nhosoi
nhosoi committed May 15, 2014
1 parent c13feee commit e9f86da
Show file tree
Hide file tree
Showing 15 changed files with 220 additions and 195 deletions.
28 changes: 14 additions & 14 deletions ldap/servers/slapd/back-ldbm/ancestorid.c
View file
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 */
Expand Down
34 changes: 17 additions & 17 deletions ldap/servers/slapd/back-ldbm/filterindex.c
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 */
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -828,7 +828,7 @@ list_candidates(
{
LDAPDebug( LDAP_DEBUG_TRACE,
"<= list_candidates NULL\n", 0, 0, 0 );
idl_free( idl );
idl_free( &idl );
idl = NULL;
goto out;
}
Expand All @@ -838,7 +838,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;
}
Expand All @@ -862,15 +862,15 @@ list_candidates(
int notin_result = 0;
notin_result = idl_notin( be, idl, tmp, &new_idl );
if (notin_result) {
idl_free(idl);
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))
Expand All @@ -880,8 +880,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. :(
*/
Expand All @@ -890,7 +890,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 );
}
}
Expand Down Expand Up @@ -1013,7 +1013,7 @@ keys2idl(
}
#endif
if ( idl2 == NULL ) {
idl_free( idl );
idl_free( &idl );
idl = NULL;
break;
}
Expand All @@ -1025,8 +1025,8 @@ keys2idl(

tmp = idl;
idl = idl_intersection(be, idl, idl2);
idl_free( idl2 );
idl_free( tmp );
idl_free( &idl2 );
idl_free( &tmp );
if ( idl == NULL ) {
break;
}
Expand Down

0 comments on commit e9f86da

Please sign in to comment.