From 13e35a48c22cef7e68b39dbf88f99631ef5f590c Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Thu, 18 Sep 2014 10:52:07 -0400 Subject: [PATCH 261/262] Ticket 47750 - Entry cache part 2 Description: replacing PRLock for cache->c_mutex with PRMonitor. Introducing cache_is_in_cache and replacing in_cache local variables (e.g., e_in_cache) with the function call. (cherry picked from commit 2c16d7d536481687b39f0d72c235aa8b7e7ef2d6) --- ldap/servers/slapd/back-ldbm/back-ldbm.h | 3 +- ldap/servers/slapd/back-ldbm/cache.c | 183 +++++++++++++++---------- ldap/servers/slapd/back-ldbm/ldbm_add.c | 134 +++++++++--------- ldap/servers/slapd/back-ldbm/ldbm_delete.c | 112 ++++++--------- ldap/servers/slapd/back-ldbm/ldbm_modify.c | 84 ++++++------ ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 147 ++++++++------------ ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 + 7 files changed, 328 insertions(+), 337 deletions(-) diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h index d27a664..ec0e2dc 100644 --- a/ldap/servers/slapd/back-ldbm/back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h @@ -402,7 +402,7 @@ struct cache { Slapi_Counter *c_tries; struct backcommon *c_lruhead; /* add entries here */ struct backcommon *c_lrutail; /* remove entries here */ - PRLock *c_mutex; /* lock for cache operations */ + PRMonitor *c_mutex; /* lock for cache operations */ PRLock *c_emutexalloc_mutex; }; @@ -694,7 +694,6 @@ typedef struct _import_subcount_stuff import_subcount_stuff; /* Handy structures for modify operations */ struct _modify_context { - int new_entry_in_cache; struct backentry *old_entry; struct backentry *new_entry; Slapi_Mods *smods; diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 5510a9a..202386e 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -543,8 +543,8 @@ int cache_init(struct cache *cache, size_t maxsize, long maxentries, int type) cache->c_lruhead = cache->c_lrutail = NULL; cache_make_hashes(cache, type); - if (((cache->c_mutex = PR_NewLock()) == NULL) || - ((cache->c_emutexalloc_mutex = PR_NewLock()) == NULL)) { + if (((cache->c_mutex = PR_NewMonitor()) == NULL) || + ((cache->c_emutexalloc_mutex = PR_NewLock()) == NULL)) { LDAPDebug(LDAP_DEBUG_ANY, "ldbm: cache_init: PR_NewLock failed\n", 0, 0, 0); return 0; @@ -633,13 +633,13 @@ static void entrycache_clear_int(struct cache *cache) void cache_clear(struct cache *cache, int type) { - PR_Lock(cache->c_mutex); + cache_lock(cache); if (CACHE_TYPE_ENTRY == type) { entrycache_clear_int(cache); } else if (CACHE_TYPE_DN == type) { dncache_clear_int(cache); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } static void erase_cache(struct cache *cache, int type) @@ -663,7 +663,7 @@ void cache_destroy_please(struct cache *cache, int type) slapi_counter_destroy(&cache->c_cursize); slapi_counter_destroy(&cache->c_hits); slapi_counter_destroy(&cache->c_tries); - PR_DestroyLock(cache->c_mutex); + PR_DestroyMonitor(cache->c_mutex); PR_DestroyLock(cache->c_emutexalloc_mutex); } @@ -687,7 +687,7 @@ static void entrycache_set_max_size(struct cache *cache, size_t bytes) "WARNING -- Minimum cache size is %lu -- rounding up\n", MINCACHESIZE, 0, 0); } - PR_Lock(cache->c_mutex); + cache_lock(cache); cache->c_maxsize = bytes; LOG("entry cache size set to %lu\n", bytes, 0, 0); /* check for full cache, and clear out if necessary */ @@ -706,7 +706,7 @@ static void entrycache_set_max_size(struct cache *cache, size_t bytes) erase_cache(cache, CACHE_TYPE_ENTRY); cache_make_hashes(cache, CACHE_TYPE_ENTRY); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); if (! dblayer_is_cachesize_sane(&bytes)) { LDAPDebug(LDAP_DEBUG_ANY, "WARNING -- Possible CONFIGURATION ERROR -- cachesize " @@ -724,7 +724,7 @@ void cache_set_max_entries(struct cache *cache, long entries) * was given in # entries instead of memory footprint. hopefully, * we can eventually drop this. */ - PR_Lock(cache->c_mutex); + cache_lock(cache); cache->c_maxentries = entries; if (entries >= 0) { LOG("entry cache entry-limit set to %lu\n", entries, 0, 0); @@ -735,7 +735,7 @@ void cache_set_max_entries(struct cache *cache, long entries) /* check for full cache, and clear out if necessary */ if (CACHE_FULL(cache)) eflush = entrycache_flush(cache); - PR_Unlock(cache->c_mutex); + cache_unlock(cache); while (eflush) { eflushtemp = BACK_LRU_NEXT(eflush, struct backentry *); @@ -748,9 +748,9 @@ size_t cache_get_max_size(struct cache *cache) { size_t n = 0; - PR_Lock(cache->c_mutex); + cache_lock(cache); n = cache->c_maxsize; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return n; } @@ -758,9 +758,9 @@ long cache_get_max_entries(struct cache *cache) { long n; - PR_Lock(cache->c_mutex); + cache_lock(cache); n = cache->c_maxentries; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return n; } @@ -786,14 +786,14 @@ void cache_get_stats(struct cache *cache, PRUint64 *hits, PRUint64 *tries, long *nentries, long *maxentries, size_t *size, size_t *maxsize) { - PR_Lock(cache->c_mutex); + cache_lock(cache); if (hits) *hits = slapi_counter_get_value(cache->c_hits); if (tries) *tries = slapi_counter_get_value(cache->c_tries); if (nentries) *nentries = cache->c_curentries; if (maxentries) *maxentries = cache->c_maxentries; if (size) *size = slapi_counter_get_value(cache->c_cursize); if (maxsize) *maxsize = cache->c_maxsize; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } void cache_debug_hash(struct cache *cache, char **out) @@ -804,7 +804,7 @@ void cache_debug_hash(struct cache *cache, char **out) Hashtable *ht = NULL; char *name = "unknown"; - PR_Lock(cache->c_mutex); + cache_lock(cache); *out = (char *)slapi_ch_malloc(1024); **out = 0; @@ -840,7 +840,7 @@ void cache_debug_hash(struct cache *cache, char **out) sprintf(*out + strlen(*out), "%d[%d] ", j, slot_stats[j]); slapi_ch_free((void **)&slot_stats); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } @@ -946,14 +946,14 @@ int cache_remove(struct cache *cache, void *ptr) } e = (struct backcommon *)ptr; - PR_Lock(cache->c_mutex); + cache_lock(cache); if (CACHE_TYPE_ENTRY == e->ep_type) { ASSERT(e->ep_refcnt > 0); ret = entrycache_remove_int(cache, (struct backentry *)e); } else if (CACHE_TYPE_DN == e->ep_type) { ret = dncache_remove_int(cache, (struct backdn *)e); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return ret; } @@ -1011,7 +1011,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, newuuid = slapi_entry_get_uniqueid(newe->ep_entry); #endif newndn = slapi_sdn_get_ndn(backentry_get_sdn(newe)); - PR_Lock(cache->c_mutex); + cache_lock(cache); /* * First, remove the old entry from all the hashtables. @@ -1031,7 +1031,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, } /* If fails, we have to make sure the both entires are removed from the cache, * otherwise, we have no idea what's left in the cache or not... */ - if (!entry_same_dn(newe, (void *)oldndn) && (newe->ep_state & ENTRY_STATE_NOTINCACHE) == 0) { + if (cache_is_in_cache(cache, newe)) { /* if we're doing a modrdn or turning an entry to a tombstone, * the new entry can be in the dn table already, so we need to remove that too. */ @@ -1039,6 +1039,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, { slapi_counter_subtract(cache->c_cursize, newe->ep_size); cache->c_curentries--; + newe->ep_refcnt--; LOG("entry cache replace remove entry size %lu\n", newe->ep_size, 0, 0); } } @@ -1061,7 +1062,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, LOG("entry cache replace (%s): cache index tables out of sync - found dn [%d] id [%d]\n", oldndn, found_in_dn, found_in_id); #endif - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } } @@ -1072,7 +1073,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, if (!add_hash(cache->c_dntable, (void *)newndn, strlen(newndn), newe, (void **)&alte)) { LOG("entry cache replace (%s): can't add to dn table (returned %s)\n", newndn, alte?slapi_entry_get_dn(alte->ep_entry):"none", 0); - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } if (!add_hash(cache->c_idtable, &(newe->ep_id), sizeof(ID), newe, (void **)&alte)) { @@ -1081,7 +1082,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, if(remove_hash(cache->c_dntable, (void *)newndn, strlen(newndn)) == 0){ LOG("entry cache replace: failed to remove dn table\n", 0, 0, 0); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } #ifdef UUIDCACHE_ON @@ -1094,7 +1095,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, if(remove_hash(cache->c_idtable, &(newe->ep_id), sizeof(ID)) == 0){ LOG("entry cache replace: failed to remove id table(uuid cache)\n", 0, 0, 0); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } #endif @@ -1107,7 +1108,7 @@ static int entrycache_replace(struct cache *cache, struct backentry *olde, slapi_counter_subtract(cache->c_cursize, olde->ep_size - newe->ep_size); } newe->ep_state = 0; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= entrycache_replace OK, cache size now %lu cache count now %ld\n", slapi_counter_get_value(cache->c_cursize), cache->c_curentries, 0); return 0; @@ -1144,7 +1145,7 @@ entrycache_return(struct cache *cache, struct backentry **bep) LOG("=> entrycache_return (%s) entry count: %d, entry in cache:%ld\n", backentry_get_ndn(e), e->ep_refcnt, cache->c_curentries); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (e->ep_state & ENTRY_STATE_NOTINCACHE) { backentry_free(bep); @@ -1163,7 +1164,7 @@ entrycache_return(struct cache *cache, struct backentry **bep) } } } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); while (eflush) { eflushtemp = BACK_LRU_NEXT(eflush, struct backentry *); @@ -1182,22 +1183,22 @@ struct backentry *cache_find_dn(struct cache *cache, const char *dn, unsigned lo LOG("=> cache_find_dn (%s)\n", dn, 0, 0); /*entry normalized by caller (dn2entry.c) */ - PR_Lock(cache->c_mutex); + cache_lock(cache); if (find_hash(cache->c_dntable, (void *)dn, ndnlen, (void **)&e)) { /* need to check entry state */ if (e->ep_state != 0) { /* entry is deleted or not fully created yet */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= cache_find_dn (NOT FOUND)\n", 0, 0, 0); return NULL; } if (e->ep_refcnt == 0) lru_delete(cache, (void *)e); e->ep_refcnt++; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); slapi_counter_increment(cache->c_hits); } else { - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } slapi_counter_increment(cache->c_tries); @@ -1213,22 +1214,22 @@ struct backentry *cache_find_id(struct cache *cache, ID id) LOG("=> cache_find_id (%lu)\n", (u_long)id, 0, 0); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (find_hash(cache->c_idtable, &id, sizeof(ID), (void **)&e)) { /* need to check entry state */ if (e->ep_state != 0) { /* entry is deleted or not fully created yet */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= cache_find_id (NOT FOUND)\n", 0, 0, 0); return NULL; } if (e->ep_refcnt == 0) lru_delete(cache, (void *)e); e->ep_refcnt++; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); slapi_counter_increment(cache->c_hits); } else { - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } slapi_counter_increment(cache->c_tries); @@ -1244,22 +1245,22 @@ struct backentry *cache_find_uuid(struct cache *cache, const char *uuid) LOG("=> cache_find_uuid (%s)\n", uuid, 0, 0); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (find_hash(cache->c_uuidtable, uuid, strlen(uuid), (void **)&e)) { /* need to check entry state */ if (e->ep_state != 0) { /* entry is deleted or not fully created yet */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= cache_find_uuid (NOT FOUND)\n", 0, 0, 0); return NULL; } if (e->ep_refcnt == 0) lru_delete(cache, (void *)e); e->ep_refcnt++; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); slapi_counter_increment(cache->c_hits); } else { - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } slapi_counter_increment(cache->c_tries); @@ -1285,7 +1286,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, LOG("=> entrycache_add_int( \"%s\", %ld )\n", backentry_get_ndn(e), e->ep_id, 0); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (! add_hash(cache->c_dntable, (void *)ndn, strlen(ndn), e, (void **)&my_alt)) { LOG("entry \"%s\" already in dn cache\n", ndn, 0, 0); @@ -1320,7 +1321,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, * to prevent that the caller accidentally thinks the existing * entry is not the same one the caller has and releases it. */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } } @@ -1330,7 +1331,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, { LOG("the entry %s is reserved (ep_state: 0x%x, state: 0x%x\n", ndn, e->ep_state, state); e->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return -1; } else if (state != 0) @@ -1338,7 +1339,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, LOG("the entry %s already exists. cannot reserve it. (ep_state: 0x%x, state: 0x%x\n", ndn, e->ep_state, state); e->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return -1; } else @@ -1350,12 +1351,14 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, (*alt)->ep_refcnt++; LOG("the entry %s already exists. returning existing entry %s (state: 0x%x)\n", ndn, backentry_get_ndn(my_alt), state); + cache_unlock(cache); + return -1; } else { LOG("the entry %s already exists. Not returning existing entry %s (state: 0x%x)\n", ndn, backentry_get_ndn(my_alt), state); + cache_unlock(cache); + return -1; } - PR_Unlock(cache->c_mutex); - return 1; } } } @@ -1385,14 +1388,14 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, * fine (i think). */ LOG("<= entrycache_add_int (ignoring)\n", 0, 0, 0); - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 0; } if(remove_hash(cache->c_dntable, (void *)ndn, strlen(ndn)) == 0){ LOG("entrycache_add_int: failed to remove %s from dn table\n", 0, 0, 0); } e->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("entrycache_add_int: failed to add %s to cache (ep_state: %x, already_in: %d)\n", ndn, e->ep_state, already_in); return -1; @@ -1411,7 +1414,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, LOG("entrycache_add_int: failed to remove id table(uuid cache)\n", 0, 0, 0); } e->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return -1; } } @@ -1437,7 +1440,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, if (CACHE_FULL(cache)) eflush = entrycache_flush(cache); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); while (eflush) { @@ -1489,11 +1492,11 @@ int cache_add_tentative(struct cache *cache, struct backentry *e, } void cache_lock(struct cache *cache) { - PR_Lock(cache->c_mutex); + PR_EnterMonitor(cache->c_mutex); } void cache_unlock(struct cache *cache) { - PR_Unlock(cache->c_mutex); + PR_ExitMonitor(cache->c_mutex); } /* locks an entry so that it can be modified (you should have gotten the @@ -1527,14 +1530,14 @@ int cache_lock_entry(struct cache *cache, struct backentry *e) PR_EnterMonitor(e->ep_mutexp); /* make sure entry hasn't been deleted now */ - PR_Lock(cache->c_mutex); + cache_lock(cache); if (e->ep_state & (ENTRY_STATE_DELETED|ENTRY_STATE_NOTINCACHE)) { - PR_Unlock(cache->c_mutex); + cache_unlock(cache); PR_ExitMonitor(e->ep_mutexp); LOG("<= cache_lock_entry (DELETED)\n", 0, 0, 0); return RETRY_CACHE_LOCK; } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= cache_lock_entry (FOUND)\n", 0, 0, 0); return 0; @@ -1600,7 +1603,7 @@ dncache_set_max_size(struct cache *cache, size_t bytes) "WARNING -- Minimum cache size is %lu -- rounding up\n", MINCACHESIZE, 0, 0); } - PR_Lock(cache->c_mutex); + cache_lock(cache); cache->c_maxsize = bytes; LOG("entry cache size set to %lu\n", bytes, 0, 0); /* check for full cache, and clear out if necessary */ @@ -1620,7 +1623,7 @@ dncache_set_max_size(struct cache *cache, size_t bytes) erase_cache(cache, CACHE_TYPE_DN); cache_make_hashes(cache, CACHE_TYPE_DN); } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); if (! dblayer_is_cachesize_sane(&bytes)) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "WARNING -- Possible CONFIGURATION ERROR -- cachesize " @@ -1684,7 +1687,7 @@ dncache_return(struct cache *cache, struct backdn **bdn) LOG("=> dncache_return (%s) reference count: %d, dn in cache:%ld\n", slapi_sdn_get_dn((*bdn)->dn_sdn), (*bdn)->ep_refcnt, cache->c_curentries); - PR_Lock(cache->c_mutex); + cache_lock(cache); if ((*bdn)->ep_state & ENTRY_STATE_NOTINCACHE) { backdn_free(bdn); @@ -1704,7 +1707,7 @@ dncache_return(struct cache *cache, struct backdn **bdn) } } } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); while (dnflush) { dnflushtemp = BACK_LRU_NEXT(dnflush, struct backdn *); @@ -1725,22 +1728,22 @@ dncache_find_id(struct cache *cache, ID id) LOG("=> dncache_find_id (%lu)\n", (u_long)id, 0, 0); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (find_hash(cache->c_idtable, &id, sizeof(ID), (void **)&bdn)) { /* need to check entry state */ if (bdn->ep_state != 0) { /* entry is deleted or not fully created yet */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= dncache_find_id (NOT FOUND)\n", 0, 0, 0); return NULL; } if (bdn->ep_refcnt == 0) lru_delete(cache, (void *)bdn); bdn->ep_refcnt++; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); slapi_counter_increment(cache->c_hits); } else { - PR_Unlock(cache->c_mutex); + cache_unlock(cache); } slapi_counter_increment(cache->c_tries); @@ -1765,7 +1768,7 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, LOG("=> dncache_add_int( \"%s\", %ld )\n", slapi_sdn_get_dn(bdn->dn_sdn), bdn->ep_id, 0); - PR_Lock(cache->c_mutex); + cache_lock(cache); if (! add_hash(cache->c_idtable, &(bdn->ep_id), sizeof(ID), bdn, (void **)&my_alt)) { @@ -1800,7 +1803,7 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, * to prevent that the caller accidentally thinks the existing * entry is not the same one the caller has and releases it. */ - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } } @@ -1810,14 +1813,14 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, { LOG("the entry is reserved\n", 0, 0, 0); bdn->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return -1; } else if (state != 0) { LOG("the entry already exists. cannot reserve it.\n", 0, 0, 0); bdn->ep_state |= ENTRY_STATE_NOTINCACHE; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return -1; } else @@ -1828,7 +1831,7 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, lru_delete(cache, (void *)*alt); (*alt)->ep_refcnt++; } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } } @@ -1857,7 +1860,7 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, dnflush = dncache_flush(cache); } } - PR_Unlock(cache->c_mutex); + cache_unlock(cache); while (dnflush) { @@ -1885,7 +1888,7 @@ dncache_replace(struct cache *cache, struct backdn *olddn, struct backdn *newdn) * where the entry isn't in all the table yet, so we don't care if any * of these return errors. */ - PR_Lock(cache->c_mutex); + cache_lock(cache); /* * First, remove the old entry from the hashtable. @@ -1897,7 +1900,7 @@ dncache_replace(struct cache *cache, struct backdn *olddn, struct backdn *newdn) found = remove_hash(cache->c_idtable, &(olddn->ep_id), sizeof(ID)); if (!found) { LOG("dn cache replace: cache index tables out of sync\n", 0, 0, 0); - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } } @@ -1908,7 +1911,7 @@ dncache_replace(struct cache *cache, struct backdn *olddn, struct backdn *newdn) */ if (!add_hash(cache->c_idtable, &(newdn->ep_id), sizeof(ID), newdn, NULL)) { LOG("dn cache replace: can't add id\n", 0, 0, 0); - PR_Unlock(cache->c_mutex); + cache_unlock(cache); return 1; } /* adjust cache meta info */ @@ -1923,7 +1926,7 @@ dncache_replace(struct cache *cache, struct backdn *olddn, struct backdn *newdn) } olddn->ep_state = ENTRY_STATE_DELETED; newdn->ep_state = 0; - PR_Unlock(cache->c_mutex); + cache_unlock(cache); LOG("<= dncache_replace OK, cache size now %lu cache count now %ld\n", slapi_counter_get_value(cache->c_cursize), cache->c_curentries, 0); return 0; @@ -2006,3 +2009,37 @@ dn_lru_verify(struct cache *cache, struct backdn *dn, int in) ASSERT(is_in == in); } #endif + +int +cache_has_otherref(struct cache *cache, void *ptr) +{ + struct backcommon *bep; + int hasref = 0; + + if (NULL == ptr) { + return hasref; + } + bep = (struct backcommon *)ptr; + cache_lock(cache); + hasref = bep->ep_refcnt; + cache_unlock(cache); + + return (hasref>1)?1:0; +} + +int +cache_is_in_cache(struct cache *cache, void *ptr) +{ + struct backcommon *bep; + int in_cache = 0; + + if (NULL == ptr) { + return in_cache; + } + bep = (struct backcommon *)ptr; + cache_lock(cache); + in_cache = (bep->ep_state & (ENTRY_STATE_DELETED|ENTRY_STATE_NOTINCACHE))?0:1; + cache_unlock(cache); + + return in_cache; +} diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index 6361bc7..5069f98 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -84,6 +84,7 @@ ldbm_back_add( Slapi_PBlock *pb ) struct backentry *addingentry = NULL; struct backentry *parententry = NULL; struct backentry *originalentry = NULL; + struct backentry *tmpentry = NULL; ID pid; int isroot; char *errbuf= NULL; @@ -103,12 +104,11 @@ ldbm_back_add( Slapi_PBlock *pb ) int ruv_c_init = 0; int rc; int addingentry_id_assigned= 0; - int addingentry_in_cache= 0; - int tombstone_in_cache= 0; Slapi_DN *sdn = NULL; Slapi_DN parentsdn; Slapi_Operation *operation; int dblock_acquired= 0; + int is_remove_from_cache= 0; int is_replicated_operation= 0; int is_resurect_operation= 0; int is_tombstone_operation= 0; @@ -118,6 +118,7 @@ ldbm_back_add( Slapi_PBlock *pb ) entry_address addr = {0}; int parent_switched = 0; int noabort = 1; + const char *dn = NULL; slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li ); slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e ); @@ -425,7 +426,6 @@ ldbm_back_add( Slapi_PBlock *pb ) ldap_result_code= -1; goto error_return; /* error result sent by find_entry2modify() */ } - tombstone_in_cache = 1; addingentry = backentry_dup( tombstoneentry ); if ( addingentry==NULL ) @@ -684,14 +684,13 @@ ldbm_back_add( Slapi_PBlock *pb ) /* Tentatively add the entry to the cache. We do this after adding any * operational attributes to ensure that the cache is sized correctly. */ - if ( cache_add_tentative( &inst->inst_cache, addingentry, NULL )!= 0 ) + if ( cache_add_tentative( &inst->inst_cache, addingentry, NULL ) < 0 ) { LDAPDebug1Arg(LDAP_DEBUG_CACHE, "cache_add_tentative concurrency detected: %s\n", slapi_entry_get_dn_const(addingentry->ep_entry)); ldap_result_code= LDAP_ALREADY_EXISTS; goto error_return; } - addingentry_in_cache = 1; /* * Before we add the entry, find out if the syntax of the aci @@ -740,33 +739,32 @@ ldbm_back_add( Slapi_PBlock *pb ) noabort = 1; slapi_pblock_set(pb, SLAPI_TXN, parent_txn); - if (addingentry_in_cache) { + /* must duplicate addingentry before returning it to cache, + * which could free the entry. */ + if ( (tmpentry = backentry_dup( addingentry )) == NULL ) { + ldap_result_code= LDAP_OPERATIONS_ERROR; + goto error_return; + } + if (cache_is_in_cache(&inst->inst_cache, addingentry)) { /* addingentry is in cache. Remove it once. */ retval = CACHE_REMOVE(&inst->inst_cache, addingentry); if (retval) { LDAPDebug1Arg(LDAP_DEBUG_CACHE, "ldbm_add: cache_remove %s failed.\n", slapi_entry_get_dn_const(addingentry->ep_entry)); } - CACHE_RETURN(&inst->inst_cache, &addingentry); - } else { - backentry_free(&addingentry); } + CACHE_RETURN(&inst->inst_cache, &addingentry); slapi_pblock_set( pb, SLAPI_ADD_ENTRY, originalentry->ep_entry ); addingentry = originalentry; - if ( (originalentry = backentry_dup( addingentry )) == NULL ) { - ldap_result_code= LDAP_OPERATIONS_ERROR; + originalentry = tmpentry; + tmpentry = NULL; + /* Adding the resetted addingentry to the cache. */ + if (cache_add_tentative(&inst->inst_cache, addingentry, NULL) < 0) { + LDAPDebug1Arg(LDAP_DEBUG_CACHE, "cache_add_tentative concurrency detected: %s\n", + slapi_entry_get_dn_const(addingentry->ep_entry)); + ldap_result_code = LDAP_ALREADY_EXISTS; goto error_return; } - if (addingentry_in_cache) { - /* Adding the resetted addingentry to the cache. */ - if (cache_add_tentative(&inst->inst_cache, addingentry, NULL) != 0) { - LDAPDebug1Arg(LDAP_DEBUG_CACHE, "cache_add_tentative concurrency detected: %s\n", - slapi_entry_get_dn_const(addingentry->ep_entry)); - ldap_result_code = LDAP_ALREADY_EXISTS; - addingentry_in_cache = 0; - goto error_return; - } - } if (ruv_c_init) { /* reset the ruv txn stuff */ modify_term(&ruv_c, be); @@ -1038,10 +1036,6 @@ ldbm_back_add( Slapi_PBlock *pb ) retval = -1; goto error_return; } - if (addingentry_in_cache) { /* decrease the refcnt added by tentative */ - CACHE_RETURN( &inst->inst_cache, &addingentry ); - } - addingentry_in_cache = 1; /* reset it to make it sure... */ /* * The tombstone was locked down in the cache... we can * get rid of the entry in the cache now. @@ -1056,9 +1050,6 @@ ldbm_back_add( Slapi_PBlock *pb ) CACHE_RETURN(&inst->inst_dncache, &bdn); } } - cache_unlock_entry( &inst->inst_cache, tombstoneentry ); - CACHE_RETURN( &inst->inst_cache, &tombstoneentry ); - tombstone_in_cache = 0; } if (parent_found) { @@ -1116,20 +1107,6 @@ error_return: { next_id_return( be, addingentry->ep_id ); } - if ( addingentry ) - { - if ( addingentry_in_cache ) - { - CACHE_REMOVE(&inst->inst_cache, addingentry); - } - else - { - if (!is_resurect_operation) { /* if resurect, tombstoneentry is dupped. */ - backentry_clear_entry(addingentry); /* e is released in the frontend */ - } - backentry_free( &addingentry ); /* release the backend wrapper, here */ - } - } if (rc == DB_RUNRECOVERY) { dblayer_remember_disk_filled(li); ldbm_nasty("Add",80,rc); @@ -1149,6 +1126,21 @@ error_return: diskfull_return: if (disk_full) { + if ( addingentry ) + { + if (inst && cache_is_in_cache(&inst->inst_cache, addingentry)) { + CACHE_REMOVE(&inst->inst_cache, addingentry); + /* tell frontend not to free this entry */ + slapi_pblock_set(pb, SLAPI_ADD_ENTRY, NULL); + } + else if (!cache_has_otherref(&inst->inst_cache, addingentry)) + { + if (!is_resurect_operation) { /* if resurect, tombstoneentry is dupped. */ + backentry_clear_entry(addingentry); /* e is released in the frontend */ + } + } + CACHE_RETURN( &inst->inst_cache, &addingentry ); + } rc = return_on_disk_full(li); } else { /* It is safer not to abort when the transaction is not started. */ @@ -1187,38 +1179,57 @@ diskfull_return: } /* txn is no longer valid - reset the txn pointer to the parent */ slapi_pblock_set(pb, SLAPI_TXN, parent_txn); + } else { + if ( addingentry ) { + if (inst && cache_is_in_cache(&inst->inst_cache, addingentry)) { + CACHE_REMOVE(&inst->inst_cache, addingentry); + /* tell frontend not to free this entry */ + slapi_pblock_set(pb, SLAPI_ADD_ENTRY, NULL); + } + else if (!cache_has_otherref(&inst->inst_cache, addingentry)) + { + if (!is_resurect_operation) { /* if resurect, tombstoneentry is dupped. */ + backentry_clear_entry(addingentry); /* e is released in the frontend */ + } + } + CACHE_RETURN( &inst->inst_cache, &addingentry ); + } } rc = SLAPI_FAIL_GENERAL; } common_return: if (inst) { - if(tombstone_in_cache && tombstoneentry) { + if (tombstoneentry && cache_is_in_cache(&inst->inst_cache, tombstoneentry)) { cache_unlock_entry(&inst->inst_cache, tombstoneentry); CACHE_RETURN(&inst->inst_cache, &tombstoneentry); } - } - if (addingentry_in_cache && addingentry) { - if ((0 == retval) && entryrdn_get_switch()) { /* subtree-rename: on */ - /* since the op was successful, add the addingentry's dn to the dn cache */ - struct backdn *bdn = dncache_find_id(&inst->inst_dncache, - addingentry->ep_id); - if (bdn) { /* already in the dncache */ - CACHE_RETURN(&inst->inst_dncache, &bdn); - } else { /* not in the dncache yet */ - Slapi_DN *addingsdn = slapi_sdn_dup(slapi_entry_get_sdn(addingentry->ep_entry)); - if (addingsdn) { - bdn = backdn_init(addingsdn, addingentry->ep_id, 0); - if (bdn) { - CACHE_ADD( &inst->inst_dncache, bdn, NULL ); - CACHE_RETURN(&inst->inst_dncache, &bdn); - slapi_log_error(SLAPI_LOG_CACHE, "ldbm_back_add", - "set %s to dn cache\n", slapi_sdn_get_dn(sdn)); + if (addingentry) { + if ((0 == retval) && entryrdn_get_switch()) { /* subtree-rename: on */ + /* since adding the entry to the entry cache was successful, + * let's add the dn to dncache, if not yet done. */ + struct backdn *bdn = dncache_find_id(&inst->inst_dncache, + addingentry->ep_id); + if (bdn) { /* already in the dncache */ + CACHE_RETURN(&inst->inst_dncache, &bdn); + } else { /* not in the dncache yet */ + Slapi_DN *addingsdn = slapi_sdn_dup(slapi_entry_get_sdn(addingentry->ep_entry)); + if (addingsdn) { + bdn = backdn_init(addingsdn, addingentry->ep_id, 0); + if (bdn) { + CACHE_ADD( &inst->inst_dncache, bdn, NULL ); + CACHE_RETURN(&inst->inst_dncache, &bdn); + slapi_log_error(SLAPI_LOG_CACHE, "ldbm_back_add", + "set %s to dn cache\n", dn); + } } } } + if (is_remove_from_cache) { + CACHE_REMOVE(&inst->inst_cache, addingentry); + } + CACHE_RETURN( &inst->inst_cache, &addingentry ); } - CACHE_RETURN( &inst->inst_cache, &addingentry ); } /* bepost op needs to know this result */ slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); @@ -1240,6 +1251,7 @@ common_return: slapi_send_ldap_result( pb, ldap_result_code, ldap_result_matcheddn, ldap_result_message, 0, NULL ); } backentry_free(&originalentry); + backentry_free(&tmpentry); slapi_sdn_done(&parentsdn); slapi_ch_free( (void**)&ldap_result_matcheddn ); slapi_ch_free( (void**)&errbuf ); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 32c119c..e06f241 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -60,6 +60,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) struct ldbminfo *li = NULL; struct backentry *e = NULL; struct backentry *tombstone = NULL; + struct backentry *tmptombstone = NULL; struct backentry *original_tombstone = NULL; const char *dn = NULL; back_txn txn; @@ -89,8 +90,6 @@ ldbm_back_delete( Slapi_PBlock *pb ) int delete_tombstone_entry = 0; /* We must remove the given tombstone entry from the DB */ int create_tombstone_entry = 0; /* We perform a "regular" LDAP delete but since we use */ /* replication, we must create a new tombstone entry */ - int tombstone_in_cache = 0; - int e_in_cache = 0; int remove_e_from_cache = 0; entry_address *addr; int addordel_flags = 0; /* passed to index_addordel */ @@ -99,6 +98,8 @@ ldbm_back_delete( Slapi_PBlock *pb ) Slapi_DN parentsdn; int opreturn = 0; int free_delete_existing_entry = 0; + ID ep_id = 0; + ID tomb_ep_id = 0; slapi_pblock_get( pb, SLAPI_BACKEND, &be); slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li ); @@ -194,7 +195,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) LDAPDebug0Args(LDAP_DEBUG_BACKLDBM, "ldbm_back_delete: Deleting entry is already deleted.\n"); goto error_return; /* error result sent by find_entry2modify() */ } - e_in_cache = 1; /* e is cached */ + ep_id = e->ep_id; retval = slapi_entry_has_children(e->ep_entry); if (retval) { @@ -472,6 +473,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) parentuniqueid= slapi_entry_get_uniqueid(parent_modify_c.old_entry->ep_entry); } tombstone = backentry_dup( e ); + tomb_ep_id = tombstone->ep_id; slapi_entry_set_dn(tombstone->ep_entry,tombstone_dn); /* Consumes DN */ if (entryrdn_get_switch()) /* subtree-rename: on */ { @@ -534,25 +536,20 @@ ldbm_back_delete( Slapi_PBlock *pb ) /* reset tombstone entry */ if (original_tombstone) { - if (tombstone_in_cache) { - CACHE_REMOVE(&inst->inst_cache, tombstone); - CACHE_RETURN(&inst->inst_cache, &tombstone); - tombstone = NULL; - tombstone_in_cache = 0; - } else { - backentry_free(&tombstone); - } - tombstone = original_tombstone; - if ( (original_tombstone = backentry_dup( tombstone )) == NULL ) { + /* must duplicate tombstone before returning it to cache, + * which could free the entry. */ + if ( (tmptombstone = backentry_dup( original_tombstone )) == NULL ) { ldap_result_code= LDAP_OPERATIONS_ERROR; goto error_return; } + if (cache_is_in_cache(&inst->inst_cache, tombstone)) { + CACHE_REMOVE(&inst->inst_cache, tombstone); + } + CACHE_RETURN(&inst->inst_cache, &tombstone); + tombstone = original_tombstone; + original_tombstone = tmptombstone; + tmptombstone = NULL; } - /* reset original entry in cache */ - if (!e_in_cache) { - CACHE_ADD(&inst->inst_cache, e, NULL); - e_in_cache = 1; - } if (ruv_c_init) { /* reset the ruv txn stuff */ modify_term(&ruv_c, be); @@ -647,13 +644,10 @@ ldbm_back_delete( Slapi_PBlock *pb ) * entry is removed from the cache. */ retval = cache_add_tentative(&inst->inst_cache, tombstone, NULL); - if (0 == retval) { - tombstone_in_cache = 1; - } else { + if (0 > retval) { LDAPDebug2Args(LDAP_DEBUG_ANY, "tombstone entry %s failed to add to the cache: %d\n", slapi_entry_get_dn(tombstone->ep_entry), retval); - tombstone_in_cache = 0; if (LDBM_OS_ERR_IS_DISKFULL(retval)) disk_full = 1; DEL_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count); @@ -672,13 +666,6 @@ ldbm_back_delete( Slapi_PBlock *pb ) DEL_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count); goto error_return; } - if (tombstone_in_cache) { - /* tombstone was already added to the cache via cache_add_tentative (to reserve its spot in the cache) - and/or id2entry_add - so it already had one refcount - cache_replace adds another refcount - - drop the extra ref added by cache_replace */ - CACHE_RETURN( &inst->inst_cache, &tombstone ); - } - tombstone_in_cache = 1; } else { @@ -1129,22 +1116,22 @@ ldbm_back_delete( Slapi_PBlock *pb ) /* delete from cache and clean up */ if (e) { - if (entryrdn_get_switch()) { /* subtree-rename: on */ + if (cache_is_in_cache(&inst->inst_cache, e)) { + ep_id = e->ep_id; + CACHE_REMOVE(&inst->inst_cache, e); + } + cache_unlock_entry(&inst->inst_cache, e); + CACHE_RETURN(&inst->inst_cache, &e); + e = NULL; + + if (entryrdn_get_switch() && ep_id) { /* subtree-rename: on */ /* since the op was successful, delete the tombstone dn from the dn cache */ - struct backdn *bdn = dncache_find_id(&inst->inst_dncache, e->ep_id); + struct backdn *bdn = dncache_find_id(&inst->inst_dncache, ep_id); if (bdn) { /* in the dncache, remove it. */ CACHE_REMOVE(&inst->inst_dncache, bdn); CACHE_RETURN(&inst->inst_dncache, &bdn); } } - if (e_in_cache) { - CACHE_REMOVE(&inst->inst_cache, e); - cache_unlock_entry(&inst->inst_cache, e); - CACHE_RETURN(&inst->inst_cache, &e); - } else { - cache_unlock_entry(&inst->inst_cache, e); - } - e = NULL; } if (ruv_c_init) { @@ -1168,36 +1155,23 @@ ldbm_back_delete( Slapi_PBlock *pb ) error_return: if (tombstone) { - if (entryrdn_get_switch()) { /* subtree-rename: on */ + if (cache_is_in_cache(&inst->inst_cache, tombstone)) { + tomb_ep_id = tombstone->ep_id; + } + + if (entryrdn_get_switch() && tomb_ep_id) { /* subtree-rename: on */ /* since the op was successful, add the addingentry's dn to the dn cache */ - struct backdn *bdn = dncache_find_id(&inst->inst_dncache, tombstone->ep_id); + struct backdn *bdn = dncache_find_id(&inst->inst_dncache, tomb_ep_id); if (bdn) { /* already in the dncache. Delete it. */ CACHE_REMOVE(&inst->inst_dncache, bdn); CACHE_RETURN(&inst->inst_dncache, &bdn); - } + } } - if (tombstone_in_cache) { /* successfully replaced */ + if (cache_is_in_cache(&inst->inst_cache, tombstone)) { CACHE_REMOVE( &inst->inst_cache, tombstone ); - CACHE_RETURN( &inst->inst_cache, &tombstone ); - tombstone = NULL; - tombstone_in_cache = 0; - } else { - backentry_free( &tombstone ); - } - } - - /* Need to return to cache after post op plugins are called */ - if (e) { - if (e_in_cache) { - if (remove_e_from_cache) { - /* The entry is already transformed to a tombstone. */ - CACHE_REMOVE( &inst->inst_cache, e ); - } - cache_unlock_entry( &inst->inst_cache, e ); - CACHE_RETURN( &inst->inst_cache, &e ); - } else { - cache_unlock_entry( &inst->inst_cache, e ); } + CACHE_RETURN( &inst->inst_cache, &tombstone ); + tombstone = NULL; } if (retval == DB_RUNRECOVERY) { @@ -1257,7 +1231,7 @@ common_return: for the post op plugins */ slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry ); } - if (tombstone) { + if (inst && tombstone) { if ((0 == retval) && entryrdn_get_switch()) { /* subtree-rename: on */ /* since the op was successful, add the addingentry's dn to the dn cache */ struct backdn *bdn = dncache_find_id(&inst->inst_dncache, tombstone->ep_id); @@ -1276,13 +1250,8 @@ common_return: } } } - if (tombstone_in_cache) { /* successfully replaced */ - CACHE_RETURN( &inst->inst_cache, &tombstone ); - tombstone = NULL; - tombstone_in_cache = 0; - } else { - backentry_free( &tombstone ); - } + CACHE_RETURN( &inst->inst_cache, &tombstone ); + tombstone = NULL; } /* result code could be used in the bepost plugin functions. */ @@ -1296,7 +1265,7 @@ common_return: } if (e) { - if (e_in_cache) { + if (cache_is_in_cache(&inst->inst_cache, e)) { if (remove_e_from_cache) { /* The entry is already transformed to a tombstone. */ CACHE_REMOVE( &inst->inst_cache, e ); @@ -1341,6 +1310,7 @@ diskfull_return: slapi_pblock_set(pb, SLAPI_DELETE_EXISTING_ENTRY, NULL); } backentry_free(&original_tombstone); + backentry_free(&tmptombstone); slapi_ch_free((void**)&errbuf); slapi_sdn_done(&nscpEntrySDN); slapi_ch_free_string(&e_uniqueid); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index b6a889f..4c1f33f 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -65,7 +65,6 @@ void modify_init(modify_context *mc,struct backentry *old_entry) PR_ASSERT(NULL == mc->new_entry); mc->old_entry = old_entry; - mc->new_entry_in_cache = 0; mc->attr_encrypt = 1; } @@ -96,16 +95,13 @@ int modify_term(modify_context *mc,struct backend *be) slapi_mods_free(&mc->smods); /* Unlock and return entries */ - if (NULL != mc->old_entry) { + if (mc->old_entry) { cache_unlock_entry(&inst->inst_cache, mc->old_entry); CACHE_RETURN( &(inst->inst_cache), &(mc->old_entry) ); mc->old_entry= NULL; } - if (mc->new_entry_in_cache) { - CACHE_RETURN( &(inst->inst_cache), &(mc->new_entry) ); - } else { - backentry_free(&(mc->new_entry)); - } + + CACHE_RETURN( &(inst->inst_cache), &(mc->new_entry) ); mc->new_entry= NULL; return 0; } @@ -115,11 +111,9 @@ int modify_switch_entries(modify_context *mc,backend *be) { ldbm_instance *inst = (ldbm_instance *) be->be_instance_info; int ret = 0; - if (mc->old_entry!=NULL && mc->new_entry!=NULL) { + if (mc->old_entry && mc->new_entry) { ret = cache_replace(&(inst->inst_cache), mc->old_entry, mc->new_entry); - if (ret == 0) { - mc->new_entry_in_cache = 1; - } else { + if (ret){ LDAPDebug(LDAP_DEBUG_CACHE, "modify_switch_entries: replacing %s with %s failed (%d)\n", slapi_entry_get_dn(mc->old_entry->ep_entry), slapi_entry_get_dn(mc->new_entry->ep_entry), ret); @@ -140,13 +134,19 @@ modify_unswitch_entries(modify_context *mc,backend *be) ldbm_instance *inst = (ldbm_instance *) be->be_instance_info; int ret = 0; - if (mc->old_entry!=NULL && mc->new_entry!=NULL) { + if (mc->old_entry && mc->new_entry && + cache_is_in_cache(&inst->inst_cache, mc->new_entry)) { /* switch the entries, and reset the new, new, entry */ tmp_be = mc->new_entry; mc->new_entry = mc->old_entry; mc->new_entry->ep_state = 0; - mc->new_entry->ep_refcnt = 0; - mc->new_entry_in_cache = 0; + if (cache_has_otherref(&(inst->inst_cache), mc->new_entry)) { + /* some other thread refers the entry */ + CACHE_RETURN(&(inst->inst_cache), &(mc->new_entry)); + } else { + /* don't call CACHE_RETURN, that frees the entry! */ + mc->new_entry->ep_refcnt = 0; + } mc->old_entry = tmp_be; ret = cache_replace(&(inst->inst_cache), mc->old_entry, mc->new_entry); @@ -157,9 +157,7 @@ modify_unswitch_entries(modify_context *mc,backend *be) * "old" one. modify_term() will then return the "new" entry. */ cache_unlock_entry(&inst->inst_cache, mc->new_entry); - CACHE_RETURN( &(inst->inst_cache), &(mc->old_entry) ); - mc->new_entry_in_cache = 1; - mc->old_entry = NULL; + cache_lock_entry(&inst->inst_cache, mc->old_entry); } else { LDAPDebug(LDAP_DEBUG_CACHE, "modify_unswitch_entries: replacing %s with %s failed (%d)\n", slapi_entry_get_dn(mc->old_entry->ep_entry), @@ -362,7 +360,8 @@ ldbm_back_modify( Slapi_PBlock *pb ) backend *be; ldbm_instance *inst = NULL; struct ldbminfo *li; - struct backentry *e = NULL, *ec = NULL, *original_entry = NULL; + struct backentry *e = NULL, *ec = NULL; + struct backentry *original_entry = NULL, *tmpentry = NULL;; Slapi_Entry *postentry = NULL; LDAPMod **mods = NULL; LDAPMod **mods_original = NULL; @@ -382,7 +381,6 @@ ldbm_back_modify( Slapi_PBlock *pb ) Slapi_Operation *operation; int dblock_acquired= 0; entry_address *addr; - int ec_in_cache = 0; int is_fixup_operation= 0; int is_ruv = 0; /* True if the current entry is RUV */ CSN *opcsn = NULL; @@ -551,14 +549,21 @@ ldbm_back_modify( Slapi_PBlock *pb ) slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); ldap_mods_free(mods, 1); slapi_pblock_set(pb, SLAPI_MODIFY_MODS, copy_mods(mods_original)); - /* ec is not really added to the cache until cache_replace, so we - don't have to worry about the cache here */ - backentry_free(&ec); - slapi_pblock_set( pb, SLAPI_MODIFY_EXISTING_ENTRY, original_entry->ep_entry ); - ec = original_entry; - if ( (original_entry = backentry_dup( ec )) == NULL ) { - ldap_result_code= LDAP_OPERATIONS_ERROR; - goto error_return; + /* reset ec set cache in id2entry_add_ext */ + if (ec) { + /* must duplicate ec before returning it to cache, + * which could free the entry. */ + if ( (tmpentry = backentry_dup( ec )) == NULL ) { + ldap_result_code= LDAP_OPERATIONS_ERROR; + goto error_return; + } + if (cache_is_in_cache(&inst->inst_cache, ec)) { + CACHE_REMOVE(&inst->inst_cache, ec); + } + CACHE_RETURN(&inst->inst_cache, &ec); + ec = original_entry; + original_entry = tmpentry; + tmpentry = NULL; } if (ruv_c_init) { @@ -740,7 +745,6 @@ ldbm_back_modify( Slapi_PBlock *pb ) /* lock new entry in cache to prevent usage until we are complete */ cache_lock_entry( &inst->inst_cache, ec ); - ec_in_cache = 1; postentry = slapi_entry_dup( ec->ep_entry ); slapi_pblock_set( pb, SLAPI_ENTRY_POST_OP, postentry ); @@ -841,11 +845,11 @@ error_return: } /* if ec is in cache, remove it, then add back e if we still have it */ - if (ec_in_cache) { + if (inst && cache_is_in_cache(&inst->inst_cache, ec)) { CACHE_REMOVE( &inst->inst_cache, ec ); /* if ec was in cache, e was not - add back e */ if (e) { - if (CACHE_ADD( &inst->inst_cache, e, NULL )) { + if (CACHE_ADD( &inst->inst_cache, e, NULL ) < 0) { LDAPDebug1Arg( LDAP_DEBUG_CACHE, "ldbm_modify: CACHE_ADD %s failed\n", slapi_entry_get_dn(e->ep_entry)); } @@ -855,20 +859,17 @@ error_return: common_return: slapi_mods_done(&smods); - if (ec_in_cache) - { - cache_unlock_entry( &inst->inst_cache, ec); - CACHE_RETURN( &inst->inst_cache, &ec ); - } - else - { - backentry_free(&ec); - /* if ec was not in cache, cache_replace was not done. - * i.e., e was not unlocked. */ - if (e) { + if(inst){ + if (cache_is_in_cache( &inst->inst_cache, ec)) + { + cache_unlock_entry( &inst->inst_cache, ec); + } else if (e) { + /* if ec was not in cache, cache_replace was not done. + * i.e., e was not unlocked. */ cache_unlock_entry( &inst->inst_cache, e); CACHE_RETURN( &inst->inst_cache, &e); } + CACHE_RETURN(&inst->inst_cache, &ec); } /* result code could be used in the bepost plugin functions. */ @@ -894,6 +895,7 @@ common_return: /* free our backups */ ldap_mods_free(mods_original, 1); backentry_free(&original_entry); + backentry_free(&tmpentry); slapi_ch_free_string(&errbuf); return rc; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 09d8f05..d8379ab 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -50,7 +50,7 @@ static void moddn_unlock_and_return_entry(backend *be,struct backentry **targete static int moddn_newrdn_mods(Slapi_PBlock *pb, const char *olddn, struct backentry *ec, Slapi_Mods *smods_wsi, int is_repl_op); static IDList *moddn_get_children(back_txn *ptxn, Slapi_PBlock *pb, backend *be, struct backentry *parententry, Slapi_DN *parentdn, struct backentry ***child_entries, struct backdn ***child_dns, int is_resurect_operation); static int moddn_rename_children(back_txn *ptxn, Slapi_PBlock *pb, backend *be, IDList *children, Slapi_DN *dn_parentdn, Slapi_DN *dn_newsuperiordn, struct backentry *child_entries[]); -static int modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3, int *e_in_cache, int *ec_in_cache); +static int modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3); static void mods_remove_nsuniqueid(Slapi_Mods *smods); #define MOD_SET_ERROR(rc, error, count) \ @@ -67,8 +67,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) struct ldbminfo *li; struct backentry *e= NULL; struct backentry *ec= NULL; - int ec_in_cache= 0; - int e_in_cache= 0; back_txn txn; back_txnid parent_txn; int retval = -1; @@ -83,8 +81,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) struct backentry *parententry= NULL; struct backentry *newparententry= NULL; struct backentry *original_entry = NULL; - struct backentry *original_parent = NULL; - struct backentry *original_newparent = NULL; + struct backentry *tmpentry = NULL; modify_context parent_modify_context = {0}; modify_context newparent_modify_context = {0}; modify_context ruv_c = {0}; @@ -361,7 +358,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) ldap_result_code= -1; goto error_return; /* error result sent by find_entry2modify() */ } - e_in_cache = 1; /* e is in the cache and locked */ if (slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE) ) { if (!is_resurect_operation) { ldap_result_code = LDAP_UNWILLING_TO_PERFORM; @@ -600,8 +596,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) } /* create it in the cache - prevents others from creating it */ - if (( cache_add_tentative( &inst->inst_cache, ec, NULL ) != 0 ) ) { - ec_in_cache = 0; /* not in cache */ + if (( cache_add_tentative( &inst->inst_cache, ec, NULL ) < 0 ) ) { /* allow modrdn even if the src dn and dest dn are identical */ if ( 0 != slapi_sdn_compare((const Slapi_DN *)&dn_newdn, (const Slapi_DN *)sdn) ) { @@ -616,46 +611,42 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) } /* so if the old dn is the same as the new dn, the entry will not be cached until it is replaced with cache_replace */ - } else { - ec_in_cache = 1; } /* Build the list of modifications required to the existing entry */ + slapi_mods_init(&smods_generated,4); + slapi_mods_init(&smods_generated_wsi,4); + ldap_result_code = moddn_newrdn_mods(pb, slapi_sdn_get_ndn(sdn), + ec, &smods_generated_wsi, is_replicated_operation); + if (ldap_result_code != LDAP_SUCCESS) { + if (ldap_result_code == LDAP_UNWILLING_TO_PERFORM) + ldap_result_message = "Modification of old rdn attribute type not allowed."; + goto error_return; + } + if (!entryrdn_get_switch()) /* subtree-rename: off */ { - slapi_mods_init(&smods_generated,4); - slapi_mods_init(&smods_generated_wsi,4); - ldap_result_code = moddn_newrdn_mods(pb, slapi_sdn_get_ndn(sdn), - ec, &smods_generated_wsi, is_replicated_operation); - if (ldap_result_code != LDAP_SUCCESS) { - if (ldap_result_code == LDAP_UNWILLING_TO_PERFORM) - ldap_result_message = "Modification of old rdn attribute type not allowed."; - goto error_return; - } - if (!entryrdn_get_switch()) /* subtree-rename: off */ - { - /* - * Remove the old entrydn index entry, and add the new one. - */ - slapi_mods_add( &smods_generated, LDAP_MOD_DELETE, LDBM_ENTRYDN_STR, - strlen(backentry_get_ndn(e)), backentry_get_ndn(e)); - slapi_mods_add( &smods_generated, LDAP_MOD_REPLACE, LDBM_ENTRYDN_STR, - strlen(backentry_get_ndn(ec)), backentry_get_ndn(ec)); - } - /* - * Update parentid if we have a new superior. + * Remove the old entrydn index entry, and add the new one. */ - if(slapi_sdn_get_dn(dn_newsuperiordn)!=NULL) { - char buf[40]; /* Enough for an ID */ - - if (parententry != NULL) { - sprintf( buf, "%lu", (u_long)parententry->ep_id ); - slapi_mods_add_string(&smods_generated, LDAP_MOD_DELETE, LDBM_PARENTID_STR, buf); - } - if (newparententry != NULL) { - sprintf( buf, "%lu", (u_long)newparententry->ep_id ); - slapi_mods_add_string(&smods_generated, LDAP_MOD_REPLACE, LDBM_PARENTID_STR, buf); - } + slapi_mods_add( &smods_generated, LDAP_MOD_DELETE, LDBM_ENTRYDN_STR, + strlen(backentry_get_ndn(e)), backentry_get_ndn(e)); + slapi_mods_add( &smods_generated, LDAP_MOD_REPLACE, LDBM_ENTRYDN_STR, + strlen(backentry_get_ndn(ec)), backentry_get_ndn(ec)); + } + + /* + * Update parentid if we have a new superior. + */ + if(slapi_sdn_get_dn(dn_newsuperiordn)!=NULL) { + char buf[40]; /* Enough for an ID */ + + if (parententry != NULL) { + sprintf( buf, "%lu", (u_long)parententry->ep_id ); + slapi_mods_add_string(&smods_generated, LDAP_MOD_DELETE, LDBM_PARENTID_STR, buf); + } + if (newparententry != NULL) { + sprintf( buf, "%lu", (u_long)newparententry->ep_id ); + slapi_mods_add_string(&smods_generated, LDAP_MOD_REPLACE, LDBM_PARENTID_STR, buf); } } @@ -873,26 +864,22 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) slapi_sdn_free(&dn_newsuperiordn); slapi_pblock_set(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, orig_dn_newsuperiordn); orig_dn_newsuperiordn = slapi_sdn_dup(orig_dn_newsuperiordn); - if (ec_in_cache) { - /* New entry 'ec' is in the entry cache. - * Remove it from the cache . */ + /* must duplicate ec before returning it to cache, + * which could free the entry. */ + if ( (tmpentry = backentry_dup( ec )) == NULL ) { + ldap_result_code= LDAP_OPERATIONS_ERROR; + goto error_return; + } + if (cache_is_in_cache(&inst->inst_cache, ec)) { CACHE_REMOVE(&inst->inst_cache, ec); - CACHE_RETURN(&inst->inst_cache, &ec); -#ifdef DEBUG_CACHE - PR_ASSERT(ec == NULL); -#endif - ec_in_cache = 0; - } else { - backentry_free(&ec); } - /* make sure the original entry is back in the cache if it was removed */ - if (!e_in_cache) { - if (CACHE_ADD(&inst->inst_cache, e, NULL)) { + CACHE_RETURN(&inst->inst_cache, &ec); + if (!cache_is_in_cache(&inst->inst_cache, e)) { + if (CACHE_ADD(&inst->inst_cache, e, NULL) < 0) { LDAPDebug1Arg(LDAP_DEBUG_CACHE, "ldbm_back_modrdn: CACHE_ADD %s to cache failed\n", slapi_entry_get_dn_const(e->ep_entry)); } - e_in_cache = 1; } slapi_pblock_get( pb, SLAPI_MODRDN_EXISTING_ENTRY, &ent ); if (ent && (ent != original_entry->ep_entry)) { @@ -900,16 +887,13 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) slapi_pblock_set( pb, SLAPI_MODRDN_EXISTING_ENTRY, NULL ); } ec = original_entry; - if ( (original_entry = backentry_dup( ec )) == NULL ) { - ldap_result_code= LDAP_OPERATIONS_ERROR; - goto error_return; - } + original_entry = tmpentry; + tmpentry = NULL; slapi_pblock_set( pb, SLAPI_MODRDN_EXISTING_ENTRY, original_entry->ep_entry ); free_modrdn_existing_entry = 0; /* owned by original_entry now */ - if (!ec_in_cache) { + if (!cache_is_in_cache(&inst->inst_cache, ec)) { /* Put the resetted entry 'ec' into the cache again. */ - if (cache_add_tentative( &inst->inst_cache, ec, NULL ) != 0) { - ec_in_cache = 0; /* not in cache */ + if (cache_add_tentative( &inst->inst_cache, ec, NULL ) < 0) { /* allow modrdn even if the src dn and dest dn are identical */ if ( 0 != slapi_sdn_compare((const Slapi_DN *)&dn_newdn, (const Slapi_DN *)sdn) ) { @@ -921,8 +905,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) } /* so if the old dn is the same as the new dn, the entry will not be cached until it is replaced with cache_replace */ - } else { - ec_in_cache = 1; } } @@ -979,7 +961,7 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) /* * Update the indexes for the entry. */ - retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi, &e_in_cache, &ec_in_cache); + retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi); if (DB_LOCK_DEADLOCK == retval) { /* Retry txn */ @@ -1412,16 +1394,13 @@ common_return: } /* remove the new entry from the cache if the op failed - otherwise, leave it in */ - if (ec_in_cache && ec && retval) { - CACHE_REMOVE( &inst->inst_cache, ec ); - } - if (ec_in_cache) { + if (inst && ec){ + if (retval && cache_is_in_cache(&inst->inst_cache, ec)) { + CACHE_REMOVE( &inst->inst_cache, ec ); + } CACHE_RETURN( &inst->inst_cache, &ec ); - } else { - backentry_free( &ec ); } ec = NULL; - ec_in_cache = 0; } /* put e back in the cache if the modrdn failed */ @@ -1468,8 +1447,7 @@ common_return: slapi_ch_free_string(&original_newrdn); slapi_sdn_free(&orig_dn_newsuperiordn); backentry_free(&original_entry); - backentry_free(&original_parent); - backentry_free(&original_newparent); + backentry_free(&tmpentry); slapi_entry_free(original_targetentry); if(dblock_acquired) @@ -1743,7 +1721,9 @@ mods_remove_nsuniqueid(Slapi_Mods *smods) * mods contains the list of attribute change made. */ static int -modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3, int *e_in_cache, int *ec_in_cache) +modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, + struct backentry *e, struct backentry **ec, + Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3) { backend *be; ldbm_instance *inst; @@ -1751,7 +1731,6 @@ modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbm char *msg; Slapi_Operation *operation; int is_ruv = 0; /* True if the current entry is RUV */ - int orig_ec_in_cache = 0; int cache_rc = 0; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); @@ -1759,7 +1738,6 @@ modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbm is_ruv = operation_is_flag_set(operation, OP_FLAG_REPL_RUV); inst = (ldbm_instance *) be->be_instance_info; - orig_ec_in_cache = *ec_in_cache; /* * Update the ID to Entry index. * Note that id2entry_add replaces the entry, so the Entry ID stays the same. @@ -1781,7 +1759,6 @@ modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbm LDAPDebug( LDAP_DEBUG_ANY, "modrdn_rename_entry_update_indexes: id2entry_add failed, err=%d %s\n", retval, (msg = dblayer_strerror( retval )) ? msg : "", 0 ); goto error_return; } - *ec_in_cache = 1; /* id2entry_add adds to cache if not already in */ if(smods1!=NULL && slapi_mods_get_num_mods(smods1)>0) { /* @@ -1869,12 +1846,7 @@ modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbm retval= -1; goto error_return; } - if (orig_ec_in_cache) { - /* ec was already added to the cache via cache_add_tentative (to reserve its spot in the cache) - and/or id2entry_add - so it already had one refcount - cache_replace adds another refcount - - drop the extra ref added by cache_replace */ - CACHE_RETURN( &inst->inst_cache, ec ); - } + error_return: return retval; } @@ -1909,8 +1881,6 @@ moddn_rename_child_entry( int olddncomps= 0; int need= 1; /* For the '\0' */ int i; - int e_in_cache = 1; - int ec_in_cache = 0; olddn = slapi_entry_get_dn((*ec)->ep_entry); if (NULL == olddn) { @@ -1973,8 +1943,7 @@ moddn_rename_child_entry( * Update all the indexes. */ retval = modrdn_rename_entry_update_indexes(ptxn, pb, li, e, ec, - smodsp, NULL, NULL, - &e_in_cache, &ec_in_cache); + smodsp, NULL, NULL); /* JCMREPL - Should the children get updated modifiersname and lastmodifiedtime? */ slapi_mods_done(&smods); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 65cafea..688f293 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -87,6 +87,8 @@ int cache_add_tentative(struct cache *cache, struct backentry *e, int cache_lock_entry(struct cache *cache, struct backentry *e); void cache_unlock_entry(struct cache *cache, struct backentry *e); int cache_replace(struct cache *cache, void *oldptr, void *newptr); +int cache_has_otherref(struct cache *cache, void *ptr); +int cache_is_in_cache(struct cache *cache, void *ptr); Hashtable *new_hash(u_long size, u_long offset, HashFn hfn, HashTestFn tfn); -- 1.9.3