ldap/servers/slapd/back-ldbm/cache.c | 43 ++++++++++++-
ldap/servers/slapd/back-ldbm/id2entry.c | 3
ldap/servers/slapd/back-ldbm/ldbm_search.c | 95 ++++++++++++++++++++++-------
ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 10 ---
ldap/servers/slapd/back-ldbm/sort.c | 1
ldap/servers/slapd/back-ldbm/vlv.c | 10 +--
ldap/servers/slapd/dse.c | 14 +---
7 files changed, 133 insertions(+), 43 deletions(-)
New commits:
commit da14d3c8a14b015d17f8b8bbcb16f9aa7d6ccd86
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Wed Feb 16 17:30:51 2011 -0800
Bug 676053 - export task followed by import task causes cache assertion
https://bugzilla.redhat.com/show_bug.cgi?id=676053
Description: There were 3 places where an entry was not released
by CACHE_RETURN (== not decrementing refcnt). If an entry has
positive refcnt in the entry cache, it won't be freed even if
the entry never be accessed again.
1. When a search request with VLV and/or SORT control failed.
2. When comparing entries in compare_entries_sv (sort.c), and the
second entry was not found, the first entry was not released.
3. vlv_trim_candidates_byvalue (vlv.c) retrieves entries for
performing binary search over the candidate list and put them
into the cache. They were not released.
This patch adds CACHE_RETURN call for the above cases.
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index 8e39c51..f21cd87 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -286,7 +286,6 @@ dump_hash(Hashtable *ht)
char *p;
int ids_size = 80;
- LDAPDebug0Args(LDAP_DEBUG_ANY, "entry cache:\n");
p = ep_ids;
for (i = 0; i < ht->size; i++) {
int len;
@@ -301,8 +300,8 @@ dump_hash(Hashtable *ht)
LDAPDebug1Arg(LDAP_DEBUG_ANY, "%s\n", ep_ids);
p = ep_ids; ids_size = 80;
}
- PR_snprintf(p, ids_size, "%s", ep_id);
- p += len; ids_size -= len + 1;
+ PR_snprintf(p, ids_size, "%s:", ep_id);
+ p += len + 1; ids_size -= len + 1;
} while (e = HASH_NEXT(ht, e));
}
if (p != ep_ids) {
@@ -614,6 +613,7 @@ static void entrycache_clear_int(struct cache *cache)
"entrycache_clear_int: there are still %ld entries "
"in the entry cache.\n", cache->c_curentries);
#ifdef LDAP_CACHE_DEBUG
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "ID(s) in entry cache:\n");
dump_hash(cache->c_idtable);
#endif
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c
b/ldap/servers/slapd/back-ldbm/ldbm_search.c
index 5565578..88af80f 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_search.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c
@@ -106,9 +106,24 @@ berval_done(struct berval *val)
/*
* We call this function as we exit ldbm_back_search
*/
-int ldbm_back_search_cleanup(Slapi_PBlock *pb, struct ldbminfo *li, sort_spec_thing
*sort_control, int ldap_result, char* ldap_result_description, int function_result,
Slapi_DN *sdn, struct vlv_request *vlv_request_control)
+static int
+ldbm_back_search_cleanup(Slapi_PBlock *pb,
+ struct ldbminfo *li,
+ sort_spec_thing *sort_control,
+ int ldap_result,
+ char* ldap_result_description,
+ int function_result,
+ Slapi_DN *sdn,
+ struct vlv_request *vlv_request_control,
+ struct backentry *e)
{
int estimate = 0; /* estimated search result count */
+ backend *be;
+ ldbm_instance *inst;
+
+ slapi_pblock_get( pb, SLAPI_BACKEND, &be );
+ inst = (ldbm_instance *) be->be_instance_info;
+ CACHE_RETURN(&inst->inst_cache, &e); /* NULL e is handled correctly */
if(sort_control!=NULL)
{
@@ -220,7 +235,10 @@ ldbm_back_search( Slapi_PBlock *pb )
if(r!=0)
{
/* Badly formed SORT control */
- return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_PROTOCOL_ERROR, "Sort Control", SLAPI_FAIL_GENERAL, &basesdn, NULL);
+ return ldbm_back_search_cleanup(pb, li, sort_control,
+ LDAP_PROTOCOL_ERROR, "Sort Control",
+ SLAPI_FAIL_GENERAL, &basesdn,
+ NULL, NULL);
}
/* set this operation includes the server side sorting */
operation->o_flags |= OP_FLAG_SERVER_SIDE_SORTING;
@@ -236,7 +254,10 @@ ldbm_back_search( Slapi_PBlock *pb )
if(r!=LDAP_SUCCESS)
{
/* Badly formed VLV control */
- return ldbm_back_search_cleanup(pb, li, sort_control, r, "VLV
Control", SLAPI_FAIL_GENERAL, &basesdn, &vlv_request_control);
+ return ldbm_back_search_cleanup(pb, li, sort_control,
+ r, "VLV Control",
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
{
/* Access Control Check to see if the client is allowed to use the
VLV Control. */
@@ -255,7 +276,10 @@ ldbm_back_search( Slapi_PBlock *pb )
if(r!=LDAP_SUCCESS)
{
/* Client isn't allowed to do this. */
- return ldbm_back_search_cleanup(pb, li, sort_control, r,
"VLV Control", SLAPI_FAIL_GENERAL, &basesdn, &vlv_request_control);
+ return ldbm_back_search_cleanup(pb, li, sort_control,
+ r, "VLV Control",
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
}
/*
@@ -267,7 +291,10 @@ ldbm_back_search( Slapi_PBlock *pb )
else
{
/* Can't have a VLV control without a SORT control */
- return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_SORT_CONTROL_MISSING, "VLV Control", SLAPI_FAIL_GENERAL, &basesdn,
&vlv_request_control);
+ return ldbm_back_search_cleanup(pb, li, sort_control,
+ LDAP_SORT_CONTROL_MISSING, "VLV Control",
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
}
}
@@ -320,13 +347,15 @@ ldbm_back_search( Slapi_PBlock *pb )
{
return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_UNWILLING_TO_PERFORM, ctrlstr,
- SLAPI_FAIL_GENERAL, &basesdn, &vlv_request_control);
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
else
{
return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_VIRTUAL_LIST_VIEW_ERROR, ctrlstr,
- SLAPI_FAIL_GENERAL, &basesdn, &vlv_request_control);
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
}
else
@@ -341,7 +370,8 @@ ldbm_back_search( Slapi_PBlock *pb )
sort_make_sort_response_control(pb, LDAP_UNWILLING_TO_PERFORM, NULL);
return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_UNAVAILABLE_CRITICAL_EXTENSION, ctrlstr,
- SLAPI_FAIL_GENERAL, &basesdn, &vlv_request_control);
+ SLAPI_FAIL_GENERAL, &basesdn,
+ &vlv_request_control, NULL);
}
else /* vlv and sorting are not critical, so ignore the control */
{
@@ -374,7 +404,8 @@ ldbm_back_search( Slapi_PBlock *pb )
if ( ( e = find_entry( pb, be, addr, NULL )) == NULL )
{
/* error or referral sent by find_entry */
- return ldbm_back_search_cleanup(pb, li, sort_control, -1, NULL, 1,
&basesdn, &vlv_request_control);
+ return ldbm_back_search_cleanup(pb, li, sort_control,
+ -1, NULL, 1, &basesdn, &vlv_request_control, NULL);
}
}
@@ -407,12 +438,13 @@ ldbm_back_search( Slapi_PBlock *pb )
return ldbm_back_search_cleanup(pb, li, sort_control,
vlv_rc, "VLV Control",
SLAPI_FAIL_GENERAL, &basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
case VLV_BLD_LIST_FAILED:
return ldbm_back_search_cleanup(pb, li, sort_control,
vlv_response_control.result,
NULL, SLAPI_FAIL_GENERAL,
- &basesdn, &vlv_request_control);
+ &basesdn, &vlv_request_control,
+ e);
case LDAP_SUCCESS:
/* Log to the access log the particulars of this sort request */
@@ -434,7 +466,7 @@ ldbm_back_search( Slapi_PBlock *pb )
"Sort Response Control",
SLAPI_FAIL_GENERAL,
&basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
}
}
}
@@ -447,7 +479,7 @@ ldbm_back_search( Slapi_PBlock *pb )
/* Error result sent by build_candidate_list */
return ldbm_back_search_cleanup(pb, li, sort_control, -1,
NULL, rc, &basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
}
/*
* If we're sorting then we must check what administrative
@@ -491,7 +523,7 @@ ldbm_back_search( Slapi_PBlock *pb )
{
return ldbm_back_search_cleanup(pb, li, sort_control,
r, NULL, -1, &basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
}
}
/*
@@ -509,7 +541,7 @@ ldbm_back_search( Slapi_PBlock *pb )
return ldbm_back_search_cleanup(pb, li, sort_control,
LDAP_PROTOCOL_ERROR,
"Sort Response Control", -1,
- &basesdn, &vlv_request_control);
+ &basesdn, &vlv_request_control, e);
}
}
else
@@ -551,7 +583,7 @@ ldbm_back_search( Slapi_PBlock *pb )
LDAP_PROTOCOL_ERROR,
"Sort Control", -1,
&basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
case LDAP_UNWILLING_TO_PERFORM: /* Too hard */
case LDAP_OPERATIONS_ERROR: /* Operation error */
case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */
@@ -593,7 +625,7 @@ ldbm_back_search( Slapi_PBlock *pb )
return ldbm_back_search_cleanup(pb, li, sort_control,
(abandoned?-1:LDAP_PROTOCOL_ERROR),
"Sort Response Control", -1,
- &basesdn, &vlv_request_control);
+ &basesdn, &vlv_request_control, e);
}
}
}
@@ -619,7 +651,7 @@ ldbm_back_search( Slapi_PBlock *pb )
return ldbm_back_search_cleanup(pb, li, sort_control,
vlv_response_control.result,
NULL, -1, &basesdn,
- &vlv_request_control);
+ &vlv_request_control, e);
}
}
else
@@ -638,7 +670,7 @@ ldbm_back_search( Slapi_PBlock *pb )
return ldbm_back_search_cleanup(pb, li, sort_control,
(abandoned?-1:LDAP_PROTOCOL_ERROR),
"VLV Response Control", -1,
- &basesdn, &vlv_request_control);
+ &basesdn, &vlv_request_control, e);
}
/* Log the VLV operation */
vlv_print_access_log(pb,&vlv_request_control,&vlv_response_control);
@@ -702,7 +734,7 @@ ldbm_back_search( Slapi_PBlock *pb )
/* tmp_err == -1: no error */
return ldbm_back_search_cleanup(pb, li, sort_control, tmp_err, tmp_desc,
(tmp_err == -1 ? 0 : -1), &basesdn,
- &vlv_request_control);
+ &vlv_request_control, NULL);
/* end Fix for bugid #394184 */
}
diff --git a/ldap/servers/slapd/back-ldbm/sort.c b/ldap/servers/slapd/back-ldbm/sort.c
index 7a2a9c9..2c9595d 100644
--- a/ldap/servers/slapd/back-ldbm/sort.c
+++ b/ldap/servers/slapd/back-ldbm/sort.c
@@ -632,6 +632,7 @@ static int compare_entries_sv(ID *id_a, ID *id_b, sort_spec
*s,baggage_carrier *
if (0 != err ) {
LDAPDebug(LDAP_DEBUG_TRACE,"compare_entries db err %d\n",err,0,0);
}
+ CACHE_RETURN(&inst->inst_cache,&a);
return 0;
}
/* OK, now we have the entries, so we work our way down the attribute list comparing as
we go */
diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c
index c68ce64..ea08fd6 100644
--- a/ldap/servers/slapd/back-ldbm/vlv.c
+++ b/ldap/servers/slapd/back-ldbm/vlv.c
@@ -1634,10 +1634,10 @@ retry:
{
match= sort_attr_compare((struct berval**)typedown_value,
entry_value, compare_fn);
}
- if (needFree) {
- ber_bvecfree((struct berval**)entry_value);
- entry_value = NULL;
- }
+ if (needFree) {
+ ber_bvecfree((struct berval**)entry_value);
+ entry_value = NULL;
+ }
}
else
{
@@ -1690,6 +1690,8 @@ retry:
LDAPDebug( LDAP_DEBUG_TRACE, "<= vlv_trim_candidates_byvalue:
Found. Index %lu\n",si, 0, 0 );
}
}
+ CACHE_RETURN(&(((ldbm_instance
*)be->be_instance_info)->inst_cache),
+ &e);
}
} while (!found);
ber_bvecfree((struct berval**)typedown_value);
commit 00a13794271527c7c474cd4d61f8ace5e7e7da32
Author: Noriko Hosoi <nhosoi(a)jiji.usersys.redhat.com>
Date: Mon Feb 14 15:06:57 2011 -0800
Bug 676053 - export task followed by import task causes cache assertion
https://bugzilla.redhat.com/show_bug.cgi?id=676053
Description: When Simple Paged Results is requested and a page is
returned, one entry is read ahead to check whether more entries
exist or not. The read-ahead retrieves an entry (if any) and adds
it into the entry cache. Simple Paged Results code puts the read-
ahead entry back, but there was missing to call cache_return for
the entry (that decrementing refcnt). If ldif2db.pl is called with
the cache state, it finds out the entry which is still referred.
This patch calls cache_return when the Simple Paged Results puts
the read-ahead entry back. Plus, adding a debug function dump_hash.
diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c
index 721a838..8e39c51 100644
--- a/ldap/servers/slapd/back-ldbm/cache.c
+++ b/ldap/servers/slapd/back-ldbm/cache.c
@@ -275,6 +275,42 @@ int remove_hash(Hashtable *ht, const void *key, size_t keylen)
return 0;
}
+#ifdef LDAP_CACHE_DEBUG
+void
+dump_hash(Hashtable *ht)
+{
+ u_long i;
+ void *e;
+ char ep_id[16];
+ char ep_ids[80];
+ char *p;
+ int ids_size = 80;
+
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "entry cache:\n");
+ p = ep_ids;
+ for (i = 0; i < ht->size; i++) {
+ int len;
+ e = ht->slot[i];
+ if (NULL == e) {
+ continue;
+ }
+ do {
+ PR_snprintf(ep_id, 16, "%u", ((struct backcommon *)e)->ep_id);
+ len = strlen(ep_id);
+ if (ids_size < len + 1) {
+ LDAPDebug1Arg(LDAP_DEBUG_ANY, "%s\n", ep_ids);
+ p = ep_ids; ids_size = 80;
+ }
+ PR_snprintf(p, ids_size, "%s", ep_id);
+ p += len; ids_size -= len + 1;
+ } while (e = HASH_NEXT(ht, e));
+ }
+ if (p != ep_ids) {
+ LDAPDebug1Arg(LDAP_DEBUG_ANY, "%s\n", ep_ids);
+ }
+}
+#endif
+
/* hashtable distribution stats --
* slots: # of slots in the hashtable
* total_entries: # of entries in the hashtable
@@ -574,9 +610,12 @@ static void entrycache_clear_int(struct cache *cache)
}
cache->c_maxsize = size;
if (cache->c_curentries > 0) {
- LDAPDebug1Arg(LDAP_DEBUG_ANY,
+ LDAPDebug1Arg(LDAP_DEBUG_ANY,
"entrycache_clear_int: there are still %ld entries "
- "in the entry cache. :/\n", cache->c_curentries);
+ "in the entry cache.\n", cache->c_curentries);
+#ifdef LDAP_CACHE_DEBUG
+ dump_hash(cache->c_idtable);
+#endif
}
}
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c
b/ldap/servers/slapd/back-ldbm/ldbm_search.c
index 568d32c..5565578 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_search.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c
@@ -1458,9 +1458,32 @@ bail:
void
ldbm_back_prev_search_results( Slapi_PBlock *pb )
{
+ backend *be;
+ ldbm_instance *inst;
back_search_result_set *sr;
+
+ slapi_pblock_get( pb, SLAPI_BACKEND, &be );
+ if (!be) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY,
+ "ldbm_back_prev_search_results: no backend\n");
+ return;
+ }
+ inst = (ldbm_instance *) be->be_instance_info;
+ if (!inst) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY,
+ "ldbm_back_prev_search_results: no backend
instance\n");
+ return;
+ }
slapi_pblock_get( pb, SLAPI_SEARCH_RESULT_SET, &sr );
if (sr) {
+ if (sr->sr_entry) {
+ /* The last entry should be returned to cache */
+ LDAPDebug1Arg(LDAP_DEBUG_BACKLDBM,
+ "ldbm_back_prev_search_results: returning: %s\n",
+ slapi_entry_get_dn_const(sr->sr_entry->ep_entry));
+ CACHE_RETURN (&inst->inst_cache, &(sr->sr_entry));
+ sr->sr_entry = NULL;
+ }
idl_iterator_decrement(&(sr->sr_current));
}
return;
commit f68c9e5ad8f8bff536a30e4fcdf089fd9d3c412e
Author: Noriko Hosoi <nhosoi(a)jiji.usersys.redhat.com>
Date: Thu Feb 10 16:29:23 2011 -0800
Bug 676053 - export task followed by import task causes cache assertion
https://bugzilla.redhat.com/show_bug.cgi?id=676053
Description: Task version of export had a bug in handling the busy
instance error case. When returning due to the busy error, the
function ldbm_back_ldbm2ldif reset the busy bit set by other threads.
This patch checks the special return value set in the busy error
case and resets the busy bit only when it is set by the function.
Also, this patch fixes a bug in dse_add which replaces an adding
entry with its copy in pblock and the original entry is consumed.
But the caller frees the original entry. Applying this patch,
instead of consuming the original entry, its copy is consumed and
the original adding entry stays in the pblock.
diff --git a/ldap/servers/slapd/back-ldbm/id2entry.c
b/ldap/servers/slapd/back-ldbm/id2entry.c
index c509d3a..432f80a 100644
--- a/ldap/servers/slapd/back-ldbm/id2entry.c
+++ b/ldap/servers/slapd/back-ldbm/id2entry.c
@@ -305,7 +305,8 @@ id2entry( backend *be, ID id, back_txn *txn, int *err )
goto bail;
}
- if ( (*err = dblayer_get_id2entry( be, &db )) != 0 ) {
+ *err = dblayer_get_id2entry( be, &db );
+ if ( (*err != 0) || (NULL == db) ) {
slapi_log_error(SLAPI_LOG_FATAL, ID2ENTRY,
"Could not open id2entry err %d\n", *err);
return( NULL );
diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
index a89f5e9..ccad8e6 100644
--- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
+++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c
@@ -1024,6 +1024,7 @@ bail:
* ldif file.
* (reunified at last)
*/
+#define LDBM2LDIF_BUSY (-2)
int
ldbm_back_ldbm2ldif( Slapi_PBlock *pb )
{
@@ -1147,7 +1148,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb )
LDAPDebug(LDAP_DEBUG_ANY, "ldbm: '%s' is already in the
middle"
" of another task and cannot be disturbed.\n",
inst->inst_name, 0, 0);
- return_value = -1;
+ return_value = LDBM2LDIF_BUSY;
goto bye;
}
}
@@ -1585,17 +1586,12 @@ bye:
}
}
- if (!run_from_cmdline && NULL != inst) {
+ if (!run_from_cmdline && inst && (LDBM2LDIF_BUSY != return_value)) {
instance_set_not_busy(inst);
}
ldbm_back_free_incl_excl(include_suffix, exclude_suffix);
idl_free(eargs.pre_exported_idl);
- if (inst != NULL) {
- PR_Lock(inst->inst_config_mutex);
- inst->inst_flags &= ~INST_FLAG_BUSY;
- PR_Unlock(inst->inst_config_mutex);
- }
return( return_value );
}
diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c
index c270892..d5d3e23 100644
--- a/ldap/servers/slapd/dse.c
+++ b/ldap/servers/slapd/dse.c
@@ -2066,25 +2066,21 @@ dse_add(Slapi_PBlock *pb) /* JCM There should only be one exit
point from this f
return dse_add_return(error, NULL);
}
- /* make copy for postop fns because add_entry_pb consumes e */
+ /* make copy for postop fns because add_entry_pb consumes the given entry */
e_copy = slapi_entry_dup(e);
- if ( dse_add_entry_pb(pdse, e, pb) != 0)
+ if ( dse_add_entry_pb(pdse, e_copy, pb) != 0)
{
slapi_send_ldap_result( pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL );
slapi_sdn_done(&sdn);
return dse_add_return(error, e_copy);
}
-
- /* e has been consumed, so use the copy for the post ops */
- slapi_pblock_set(pb, SLAPI_ADD_ENTRY, e_copy);
-
/* The postop must be called after the write lock is released. */
- dse_call_callback(pdse, pb, SLAPI_OPERATION_ADD, DSE_FLAG_POSTOP, e_copy, NULL,
&returncode, returntext);
+ dse_call_callback(pdse, pb, SLAPI_OPERATION_ADD, DSE_FLAG_POSTOP, e, NULL,
&returncode, returntext);
/* We have been successful. Tell the user */
slapi_send_ldap_result( pb, returncode, NULL, NULL, 0, NULL );
- slapi_pblock_set( pb, SLAPI_ENTRY_POST_OP, slapi_entry_dup( e_copy ));
+ slapi_pblock_set( pb, SLAPI_ENTRY_POST_OP, slapi_entry_dup( e ));
/* entry has been freed, so make sure no one tries to use it later */
slapi_pblock_set(pb, SLAPI_ADD_ENTRY, NULL);
@@ -2092,7 +2088,7 @@ dse_add(Slapi_PBlock *pb) /* JCM There should only be one exit point
from this f
/* Free the dn, and return */
slapi_sdn_done(&sdn);
- return dse_add_return(rc, e_copy);
+ return dse_add_return(rc, e);
}
/*