Blame SOURCES/0077-Ticket-48235-Remove-memberOf-global-lock.patch

40cd75
From 229f61f5f54aeb9e1a1756f731dfe7bcedbf148c Mon Sep 17 00:00:00 2001
40cd75
From: Mark Reynolds <mreynolds@redhat.com>
40cd75
Date: Fri, 13 Oct 2017 07:09:08 -0400
40cd75
Subject: [PATCH 06/10] Ticket 48235 - Remove memberOf global lock
40cd75
40cd75
Bug Description:  The memberOf global lock no longer servers a purpose since
40cd75
                  the plugin is BETXN.  This was causing potential deadlocks
40cd75
                  when multiple backends are used.
40cd75
40cd75
Fix Description:  Remove the lock, and rework the fixup/ancestors caches/hashtables.
40cd75
                  Instead of reusing a single cache, we create a fresh cache
40cd75
                  when we copy the plugin config (which only happens at the start
40cd75
                  of an operation).  Then we destroy the caches when we free
40cd75
                  the config.
40cd75
40cd75
https://pagure.io/389-ds-base/issue/48235
40cd75
40cd75
Reviewed by: tbordaz & firstyear(Thanks!!)
40cd75
---
40cd75
 ldap/servers/plugins/memberof/memberof.c        | 312 +++---------------------
40cd75
 ldap/servers/plugins/memberof/memberof.h        |  17 ++
40cd75
 ldap/servers/plugins/memberof/memberof_config.c | 152 +++++++++++-
40cd75
 3 files changed, 200 insertions(+), 281 deletions(-)
40cd75
40cd75
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
40cd75
index 9bbe13c9c..bbf47dd49 100644
40cd75
--- a/ldap/servers/plugins/memberof/memberof.c
40cd75
+++ b/ldap/servers/plugins/memberof/memberof.c
40cd75
@@ -49,13 +49,10 @@ static void* _PluginID = NULL;
40cd75
 static Slapi_DN* _ConfigAreaDN = NULL;
40cd75
 static Slapi_RWLock *config_rwlock = NULL;
40cd75
 static Slapi_DN* _pluginDN = NULL;
40cd75
-static PRMonitor *memberof_operation_lock = 0;
40cd75
 MemberOfConfig *qsortConfig = 0;
40cd75
 static int usetxn = 0;
40cd75
 static int premodfn = 0;
40cd75
-#define MEMBEROF_HASHTABLE_SIZE 1000
40cd75
-static PLHashTable *fixup_entry_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */
40cd75
-static PLHashTable *group_ancestors_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */
40cd75
+
40cd75
 
40cd75
 typedef struct _memberofstringll
40cd75
 {
40cd75
@@ -73,18 +70,7 @@ typedef struct _memberof_get_groups_data
40cd75
         PRBool use_cache;
40cd75
 } memberof_get_groups_data;
40cd75
 
40cd75
-/* The key to access the hash table is the normalized DN
40cd75
- * The normalized DN is stored in the value because:
40cd75
- *  - It is used in slapi_valueset_find
40cd75
- *  - It is used to fill the memberof_get_groups_data.group_norm_vals
40cd75
- */
40cd75
-typedef struct _memberof_cached_value
40cd75
-{
40cd75
-	char *key;
40cd75
-	char *group_dn_val;
40cd75
-        char *group_ndn_val;
40cd75
-	int valid;
40cd75
-} memberof_cached_value;
40cd75
+
40cd75
 struct cache_stat
40cd75
 {
40cd75
 	int total_lookup;
40cd75
@@ -189,14 +175,9 @@ static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);
40cd75
 static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);
40cd75
 static int memberof_add_objectclass(char *auto_add_oc, const char *dn);
40cd75
 static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc);
40cd75
-static PLHashTable *hashtable_new();
40cd75
-static void fixup_hashtable_empty(char *msg);
40cd75
-static PLHashTable *hashtable_new();
40cd75
-static void ancestor_hashtable_empty(char *msg);
40cd75
-static void ancestor_hashtable_entry_free(memberof_cached_value *entry);
40cd75
-static memberof_cached_value *ancestors_cache_lookup(const char *ndn);
40cd75
-static PRBool ancestors_cache_remove(const char *ndn);
40cd75
-static PLHashEntry *ancestors_cache_add(const void *key, void *value);
40cd75
+static memberof_cached_value *ancestors_cache_lookup(MemberOfConfig *config, const char *ndn);
40cd75
+static PRBool ancestors_cache_remove(MemberOfConfig *config, const char *ndn);
40cd75
+static PLHashEntry *ancestors_cache_add(MemberOfConfig *config, const void *key, void *value);
40cd75
 
40cd75
 /*** implementation ***/
40cd75
 
40cd75
@@ -375,12 +356,6 @@ int memberof_postop_start(Slapi_PBlock *pb)
40cd75
 	slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
40cd75
 		"--> memberof_postop_start\n" );
40cd75
 
40cd75
-	memberof_operation_lock = PR_NewMonitor();
40cd75
-	if(0 == memberof_operation_lock)
40cd75
-	{
40cd75
-		rc = -1;
40cd75
-		goto bail;
40cd75
-	}
40cd75
 	if(config_rwlock == NULL){
40cd75
 		if((config_rwlock = slapi_new_rwlock()) == NULL){
40cd75
 			rc = -1;
40cd75
@@ -388,9 +363,6 @@ int memberof_postop_start(Slapi_PBlock *pb)
40cd75
 		}
40cd75
 	}
40cd75
 
40cd75
-	fixup_entry_hashtable = hashtable_new();
40cd75
-	group_ancestors_hashtable = hashtable_new();
40cd75
-
40cd75
 	/* Set the alternate config area if one is defined. */
40cd75
 	slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area);
40cd75
 	if (config_area)
40cd75
@@ -482,18 +454,7 @@ int memberof_postop_close(Slapi_PBlock *pb)
40cd75
 	slapi_sdn_free(&_pluginDN);
40cd75
 	slapi_destroy_rwlock(config_rwlock);
40cd75
 	config_rwlock = NULL;
40cd75
-	PR_DestroyMonitor(memberof_operation_lock);
40cd75
-	memberof_operation_lock = NULL;
40cd75
-
40cd75
-	if (fixup_entry_hashtable) {
40cd75
-		fixup_hashtable_empty("memberof_postop_close empty fixup_entry_hastable");
40cd75
-		PL_HashTableDestroy(fixup_entry_hashtable);
40cd75
-	}
40cd75
 
40cd75
-	if (group_ancestors_hashtable) {
40cd75
-		ancestor_hashtable_empty("memberof_postop_close empty group_ancestors_hashtable");
40cd75
-		PL_HashTableDestroy(group_ancestors_hashtable);
40cd75
-	}
40cd75
 	slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
40cd75
 		     "<-- memberof_postop_close\n" );
40cd75
 	return 0;
40cd75
@@ -554,7 +515,7 @@ int memberof_postop_del(Slapi_PBlock *pb)
40cd75
 {
40cd75
 	int ret = SLAPI_PLUGIN_SUCCESS;
40cd75
 	MemberOfConfig *mainConfig = NULL;
40cd75
-	MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+	MemberOfConfig configCopy = {0};
40cd75
 	Slapi_DN *sdn;
40cd75
 	void *caller_id = NULL;
40cd75
 
40cd75
@@ -583,9 +544,6 @@ int memberof_postop_del(Slapi_PBlock *pb)
40cd75
 		}
40cd75
 		memberof_copy_config(&configCopy, memberof_get_config());
40cd75
 		memberof_unlock_config();
40cd75
-
40cd75
-		/* get the memberOf operation lock */
40cd75
-		memberof_lock();
40cd75
 		
40cd75
 		/* remove this DN from the
40cd75
 		 * membership lists of groups
40cd75
@@ -594,7 +552,6 @@ int memberof_postop_del(Slapi_PBlock *pb)
40cd75
 			slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM,
40cd75
 			                "memberof_postop_del - Error deleting dn (%s) from group. Error (%d)\n",
40cd75
 			                slapi_sdn_get_dn(sdn),ret);
40cd75
-			memberof_unlock();
40cd75
 			goto bail;
40cd75
 		}
40cd75
 
40cd75
@@ -618,7 +575,6 @@ int memberof_postop_del(Slapi_PBlock *pb)
40cd75
 				}
40cd75
 			}
40cd75
 		}
40cd75
-		memberof_unlock();
40cd75
 bail:
40cd75
 		memberof_free_config(&configCopy);
40cd75
 	}
40cd75
@@ -813,7 +769,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn
40cd75
 		memberof_cached_value *ht_grp = NULL;
40cd75
 		const char *ndn = slapi_sdn_get_ndn(sdn);
40cd75
 		
40cd75
-		ht_grp = ancestors_cache_lookup((const void *) ndn);
40cd75
+		ht_grp = ancestors_cache_lookup(config, (const void *) ndn);
40cd75
 		if (ht_grp) {
40cd75
 #if MEMBEROF_CACHE_DEBUG
40cd75
 			slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn, ht_grp);
40cd75
@@ -960,7 +916,7 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)
40cd75
 	if(memberof_oktodo(pb))
40cd75
 	{
40cd75
 		MemberOfConfig *mainConfig = 0;
40cd75
-		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+		MemberOfConfig configCopy = {0};
40cd75
 		struct slapi_entry *pre_e = NULL;
40cd75
 		struct slapi_entry *post_e = NULL;
40cd75
 		Slapi_DN *pre_sdn = 0;
40cd75
@@ -988,8 +944,6 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)
40cd75
 			goto bail;
40cd75
 		}
40cd75
 
40cd75
-		memberof_lock();
40cd75
-
40cd75
 		/*  update any downstream members */
40cd75
 		if(pre_sdn && post_sdn && configCopy.group_filter &&
40cd75
 		   0 == slapi_filter_test_simple(post_e, configCopy.group_filter))
40cd75
@@ -1060,7 +1014,6 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)
40cd75
 				}
40cd75
 			}
40cd75
 		}
40cd75
-		memberof_unlock();
40cd75
 bail:
40cd75
 		memberof_free_config(&configCopy);
40cd75
 	}
40cd75
@@ -1220,7 +1173,7 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 	{
40cd75
 		int config_copied = 0;
40cd75
 		MemberOfConfig *mainConfig = 0;
40cd75
-		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+		MemberOfConfig configCopy = {0};
40cd75
 
40cd75
 		/* get the mod set */
40cd75
 		slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods;;
40cd75
@@ -1267,8 +1220,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 			{
40cd75
 				int op = slapi_mod_get_operation(smod);
40cd75
 
40cd75
-				memberof_lock();
40cd75
-
40cd75
 				/* the modify op decides the function */
40cd75
 				switch(op & ~LDAP_MOD_BVALUES)
40cd75
 				{
40cd75
@@ -1280,7 +1231,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 								"memberof_postop_modify - Failed to add dn (%s) to target.  "
40cd75
 								"Error (%d)\n", slapi_sdn_get_dn(sdn), ret );
40cd75
 							slapi_mod_done(next_mod);
40cd75
-							memberof_unlock();
40cd75
 							goto bail;
40cd75
 						}
40cd75
 						break;
40cd75
@@ -1299,7 +1249,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 									"memberof_postop_modify - Failed to replace list (%s).  "
40cd75
 									"Error (%d)\n", slapi_sdn_get_dn(sdn), ret );
40cd75
 								slapi_mod_done(next_mod);
40cd75
-								memberof_unlock();
40cd75
 								goto bail;
40cd75
 							}
40cd75
 						}
40cd75
@@ -1311,7 +1260,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 									"memberof_postop_modify: failed to remove dn (%s).  "
40cd75
 									"Error (%d)\n", slapi_sdn_get_dn(sdn), ret );
40cd75
 								slapi_mod_done(next_mod);
40cd75
-								memberof_unlock();
40cd75
 								goto bail;
40cd75
 							}
40cd75
 						}
40cd75
@@ -1326,7 +1274,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 								"memberof_postop_modify - Failed to replace values in  dn (%s).  "
40cd75
 								"Error (%d)\n", slapi_sdn_get_dn(sdn), ret );
40cd75
 							slapi_mod_done(next_mod);
40cd75
-							memberof_unlock();
40cd75
 							goto bail;
40cd75
 						}
40cd75
 						break;
40cd75
@@ -1342,8 +1289,6 @@ int memberof_postop_modify(Slapi_PBlock *pb)
40cd75
 						break;
40cd75
 					}
40cd75
 				}
40cd75
-
40cd75
-				memberof_unlock();
40cd75
 			}
40cd75
 
40cd75
 			slapi_mod_done(next_mod);
40cd75
@@ -1398,7 +1343,7 @@ int memberof_postop_add(Slapi_PBlock *pb)
40cd75
 	if(memberof_oktodo(pb) && (sdn = memberof_getsdn(pb)))
40cd75
 	{
40cd75
 		struct slapi_entry *e = NULL;
40cd75
-		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+		MemberOfConfig configCopy = {0};
40cd75
 		MemberOfConfig *mainConfig;
40cd75
 		slapi_pblock_get( pb, SLAPI_ENTRY_POST_OP, &e );
40cd75
 
40cd75
@@ -1424,8 +1369,6 @@ int memberof_postop_add(Slapi_PBlock *pb)
40cd75
 			int i = 0;
40cd75
 			Slapi_Attr *attr = 0;
40cd75
 
40cd75
-			memberof_lock();
40cd75
-
40cd75
 			for (i = 0; configCopy.groupattrs && configCopy.groupattrs[i]; i++)
40cd75
 			{
40cd75
 				if(0 == slapi_entry_attr_find(e, configCopy.groupattrs[i], &attr))
40cd75
@@ -1438,8 +1381,6 @@ int memberof_postop_add(Slapi_PBlock *pb)
40cd75
 					}
40cd75
 				}
40cd75
 			}
40cd75
-
40cd75
-			memberof_unlock();
40cd75
 			memberof_free_config(&configCopy);
40cd75
 		}
40cd75
 	}
40cd75
@@ -2201,7 +2142,7 @@ dump_cache_entry(memberof_cached_value *double_check, const char *msg)
40cd75
  * the firsts elements of the array has 'valid=1' and the dn/ndn of group it belong to
40cd75
  */
40cd75
 static void
40cd75
-cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
40cd75
+cache_ancestors(MemberOfConfig *config, Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
40cd75
 {
40cd75
 	Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)groups)->groupvals;
40cd75
 	Slapi_Value *sval;
40cd75
@@ -2298,14 +2239,14 @@ cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
40cd75
 #if MEMBEROF_CACHE_DEBUG
40cd75
 	dump_cache_entry(cache_entry, key);
40cd75
 #endif
40cd75
-	if (ancestors_cache_add((const void*) key_copy, (void *) cache_entry) == NULL) {
40cd75
+	if (ancestors_cache_add(config, (const void*) key_copy, (void *) cache_entry) == NULL) {
40cd75
 		slapi_log_err( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Failed to cache ancestor of %s\n", key);
40cd75
 		ancestor_hashtable_entry_free(cache_entry);
40cd75
 		slapi_ch_free ((void**)&cache_entry);
40cd75
 		return;
40cd75
 	}
40cd75
 #if MEMBEROF_CACHE_DEBUG
40cd75
-	if (double_check = ancestors_cache_lookup((const void*) key)) {
40cd75
+	if (double_check = ancestors_cache_lookup(config, (const void*) key)) {
40cd75
 		dump_cache_entry(double_check, "read back");
40cd75
 	}
40cd75
 #endif
40cd75
@@ -2390,9 +2331,9 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn,
40cd75
 		memberof_get_groups_callback, &member_data, &cached, member_data.use_cache);
40cd75
 
40cd75
 	merge_ancestors(&member_ndn_val, &member_data, data);
40cd75
-	if (!cached && member_data.use_cache)
40cd75
-		cache_ancestors(&member_ndn_val, &member_data);
40cd75
-
40cd75
+	if (!cached && member_data.use_cache) {
40cd75
+		cache_ancestors(config, &member_ndn_val, &member_data);
40cd75
+	}
40cd75
 
40cd75
 	slapi_value_free(&member_ndn_val);
40cd75
 	slapi_valueset_free(groupvals);
40cd75
@@ -2969,46 +2910,9 @@ int memberof_qsort_compare(const void *a, const void *b)
40cd75
 	                                val1, val2);
40cd75
 }
40cd75
 
40cd75
-/* betxn: This locking mechanism is necessary to guarantee the memberof
40cd75
- * consistency */
40cd75
-void memberof_lock()
40cd75
-{
40cd75
-	if (usetxn) {
40cd75
-		PR_EnterMonitor(memberof_operation_lock);
40cd75
-	}
40cd75
-	if (fixup_entry_hashtable) {
40cd75
-		fixup_hashtable_empty("memberof_lock");
40cd75
-	}
40cd75
-	if (group_ancestors_hashtable) {
40cd75
-		ancestor_hashtable_empty("memberof_lock empty group_ancestors_hashtable");
40cd75
-		memset(&cache_stat, 0, sizeof(cache_stat));
40cd75
-	}
40cd75
-}
40cd75
-
40cd75
-void memberof_unlock()
40cd75
-{
40cd75
-	if (group_ancestors_hashtable) {
40cd75
-		ancestor_hashtable_empty("memberof_unlock empty group_ancestors_hashtable");
40cd75
-#if MEMBEROF_CACHE_DEBUG
40cd75
-		slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics: total lookup %d (success %d), add %d, remove %d, enum %d\n",
40cd75
-			cache_stat.total_lookup, cache_stat.successfull_lookup,
40cd75
-			cache_stat.total_add, cache_stat.total_remove, cache_stat.total_enumerate);
40cd75
-		slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics duration: lookup %ld, add %ld, remove %ld, enum %ld\n",
40cd75
-			cache_stat.cumul_duration_lookup, cache_stat.cumul_duration_add,
40cd75
-			cache_stat.cumul_duration_remove, cache_stat.cumul_duration_enumerate);
40cd75
-#endif
40cd75
-	}
40cd75
-	if (fixup_entry_hashtable) {
40cd75
-		fixup_hashtable_empty("memberof_lock");
40cd75
-	}
40cd75
-	if (usetxn) {
40cd75
-		PR_ExitMonitor(memberof_operation_lock);
40cd75
-	}
40cd75
-}
40cd75
-
40cd75
 void memberof_fixup_task_thread(void *arg)
40cd75
 {
40cd75
-	MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+	MemberOfConfig configCopy = {0};
40cd75
 	Slapi_Task *task = (Slapi_Task *)arg;
40cd75
 	task_data *td = NULL;
40cd75
 	int rc = 0;
40cd75
@@ -3068,14 +2972,8 @@ void memberof_fixup_task_thread(void *arg)
40cd75
 		}
40cd75
 	}
40cd75
 
40cd75
-	/* get the memberOf operation lock */
40cd75
-	memberof_lock();
40cd75
-
40cd75
 	/* do real work */
40cd75
 	rc = memberof_fix_memberof(&configCopy, task, td);
40cd75
- 
40cd75
-	/* release the memberOf operation lock */
40cd75
-	memberof_unlock();
40cd75
 
40cd75
 done:
40cd75
 	if (usetxn && fixup_pb) {
40cd75
@@ -3240,7 +3138,7 @@ int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *t
40cd75
 }
40cd75
 
40cd75
 static memberof_cached_value *
40cd75
-ancestors_cache_lookup(const char *ndn)
40cd75
+ancestors_cache_lookup(MemberOfConfig *config, const char *ndn)
40cd75
 {
40cd75
 	memberof_cached_value *e;
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
@@ -3258,7 +3156,7 @@ ancestors_cache_lookup(const char *ndn)
40cd75
 	}
40cd75
 #endif
40cd75
 
40cd75
-	e = (memberof_cached_value *) PL_HashTableLookupConst(group_ancestors_hashtable, (const void *) ndn);
40cd75
+	e = (memberof_cached_value *) PL_HashTableLookupConst(config->ancestors_cache, (const void *) ndn);
40cd75
 
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
 	if (start) {
40cd75
@@ -3274,7 +3172,7 @@ ancestors_cache_lookup(const char *ndn)
40cd75
 	
40cd75
 }
40cd75
 static PRBool
40cd75
-ancestors_cache_remove(const char *ndn)
40cd75
+ancestors_cache_remove(MemberOfConfig *config, const char *ndn)
40cd75
 {
40cd75
 	PRBool rc;
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
@@ -3292,7 +3190,7 @@ ancestors_cache_remove(const char *ndn)
40cd75
 	}
40cd75
 #endif
40cd75
 
40cd75
-	rc = PL_HashTableRemove(group_ancestors_hashtable, (const void *) ndn);
40cd75
+	rc = PL_HashTableRemove(config->ancestors_cache, (const void *) ndn);
40cd75
 
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
 	if (start) {
40cd75
@@ -3305,7 +3203,7 @@ ancestors_cache_remove(const char *ndn)
40cd75
 }
40cd75
 
40cd75
 static PLHashEntry *
40cd75
-ancestors_cache_add(const void *key, void *value)
40cd75
+ancestors_cache_add(MemberOfConfig *config, const void *key, void *value)
40cd75
 {
40cd75
 	PLHashEntry *e;
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
@@ -3322,7 +3220,7 @@ ancestors_cache_add(const void *key, void *value)
40cd75
 	}
40cd75
 #endif
40cd75
 
40cd75
-	e = PL_HashTableAdd(group_ancestors_hashtable, key, value);
40cd75
+	e = PL_HashTableAdd(config->ancestors_cache, key, value);
40cd75
 
40cd75
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
 	if (start) {
40cd75
@@ -3360,10 +3258,11 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
40cd75
 		goto bail;
40cd75
 	}
40cd75
 
40cd75
-        /* Check if the entry has not already been fixed */
40cd75
+    /* Check if the entry has not already been fixed */
40cd75
 	ndn = slapi_sdn_get_ndn(sdn);
40cd75
-	if (ndn && fixup_entry_hashtable && PL_HashTableLookupConst(fixup_entry_hashtable, (void*) ndn)) {
40cd75
-		slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn);
40cd75
+	if (ndn && config->fixup_cache && PL_HashTableLookupConst(config->fixup_cache, (void*) ndn)) {
40cd75
+		slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM,
40cd75
+            "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn);
40cd75
 		goto bail;
40cd75
 	}
40cd75
 
40cd75
@@ -3383,9 +3282,9 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
40cd75
 #if MEMBEROF_CACHE_DEBUG
40cd75
 			slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: This is NOT a group %s\n", ndn);
40cd75
 #endif
40cd75
-			ht_grp = ancestors_cache_lookup((const void *) ndn);
40cd75
+			ht_grp = ancestors_cache_lookup(config, (const void *) ndn);
40cd75
 			if (ht_grp) {
40cd75
-				if (ancestors_cache_remove((const void *) ndn)) {
40cd75
+				if (ancestors_cache_remove(config, (const void *) ndn)) {
40cd75
 					slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: free cached values for %s\n", ndn);
40cd75
 					ancestor_hashtable_entry_free(ht_grp);
40cd75
 					slapi_ch_free((void **) &ht_grp);
40cd75
@@ -3400,6 +3299,7 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
40cd75
 			}
40cd75
 		}
40cd75
 	}
40cd75
+
40cd75
 	/* If we found some groups, replace the existing memberOf attribute
40cd75
 	 * with the found values.  */
40cd75
 	if (groups && slapi_valueset_count(groups))
40cd75
@@ -3439,9 +3339,9 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
40cd75
 	slapi_valueset_free(groups);
40cd75
 
40cd75
 	/* records that this entry has been fixed up */
40cd75
-	if (fixup_entry_hashtable) {
40cd75
+	if (config->fixup_cache) {
40cd75
 		dn_copy = slapi_ch_strdup(ndn);
40cd75
-		if (PL_HashTableAdd(fixup_entry_hashtable, dn_copy, dn_copy) == NULL) {
40cd75
+		if (PL_HashTableAdd(config->fixup_cache, dn_copy, dn_copy) == NULL) {
40cd75
 			slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: "
40cd75
 				"failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n",
40cd75
 				dn_copy, PR_GetError());
40cd75
@@ -3539,150 +3439,8 @@ memberof_add_objectclass(char *auto_add_oc, const char *dn)
40cd75
 	return rc;
40cd75
 }
40cd75
 
40cd75
-static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2)
40cd75
-{
40cd75
-	PRIntn rc;
40cd75
-	if (0 == strcasecmp((const char *) v1, (const char *) v2)) {
40cd75
-		rc = 1;
40cd75
-	} else {
40cd75
-		rc = 0;
40cd75
-	}
40cd75
-	return rc;
40cd75
-}
40cd75
-
40cd75
-static PRIntn memberof_hash_compare_values(const void *v1, const void *v2)
40cd75
-{
40cd75
-	PRIntn rc;
40cd75
-	if ((char *) v1 == (char *) v2) {
40cd75
-		rc = 1;
40cd75
-	} else {
40cd75
-		rc = 0;
40cd75
-	}
40cd75
-	return rc;
40cd75
-}
40cd75
-
40cd75
-/*
40cd75
- *  Hashing function using Bernstein's method
40cd75
- */
40cd75
-static PLHashNumber memberof_hash_fn(const void *key)
40cd75
-{
40cd75
-    PLHashNumber hash = 5381;
40cd75
-    unsigned char *x = (unsigned char *)key;
40cd75
-    int c;
40cd75
-
40cd75
-    while ((c = *x++)){
40cd75
-        hash = ((hash << 5) + hash) ^ c;
40cd75
-    }
40cd75
-    return hash;
40cd75
-}
40cd75
-
40cd75
-/* allocates the plugin hashtable
40cd75
- * This hash table is used by operation and is protected from
40cd75
- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
40cd75
- * is not implemented and the hash table will be not used.
40cd75
- *
40cd75
- * The hash table contains all the DN of the entries for which the memberof
40cd75
- * attribute has been computed/updated during the current operation
40cd75
- *
40cd75
- * hash table should be empty at the beginning and end of the plugin callback
40cd75
- */
40cd75
-static PLHashTable *hashtable_new()
40cd75
-{
40cd75
-	if (!usetxn) {
40cd75
-		return NULL;
40cd75
-	}
40cd75
-
40cd75
-	return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
40cd75
-		memberof_hash_fn,
40cd75
-		memberof_hash_compare_keys,
40cd75
-		memberof_hash_compare_values, NULL, NULL);
40cd75
-}
40cd75
-/* this function called for each hash node during hash destruction */
40cd75
-static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg)
40cd75
-{
40cd75
-	char *dn_copy;
40cd75
-
40cd75
-	if (he == NULL) {
40cd75
-		return HT_ENUMERATE_NEXT;
40cd75
-	}
40cd75
-	dn_copy = (char*) he->value;
40cd75
-	slapi_ch_free_string(&dn_copy);
40cd75
-
40cd75
-	return HT_ENUMERATE_REMOVE;
40cd75
-}
40cd75
-
40cd75
-static void fixup_hashtable_empty(char *msg)
40cd75
-{
40cd75
-	if (fixup_entry_hashtable) {
40cd75
-		PL_HashTableEnumerateEntries(fixup_entry_hashtable, fixup_hashtable_remove, msg);
40cd75
-	}
40cd75
-}
40cd75
-
40cd75
-
40cd75
-/* allocates the plugin hashtable
40cd75
- * This hash table is used by operation and is protected from
40cd75
- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
40cd75
- * is not implemented and the hash table will be not used.
40cd75
- *
40cd75
- * The hash table contains all the DN of the entries for which the memberof
40cd75
- * attribute has been computed/updated during the current operation
40cd75
- *
40cd75
- * hash table should be empty at the beginning and end of the plugin callback
40cd75
- */
40cd75
-
40cd75
-static
40cd75
-void ancestor_hashtable_entry_free(memberof_cached_value *entry)
40cd75
-{
40cd75
-	int i;
40cd75
-	for (i = 0; entry[i].valid; i++) {
40cd75
-		slapi_ch_free((void **) &entry[i].group_dn_val);
40cd75
-		slapi_ch_free((void **) &entry[i].group_ndn_val);
40cd75
-	}
40cd75
-	/* Here we are at the ending element containing the key */
40cd75
-	slapi_ch_free((void**) &entry[i].key);
40cd75
-}
40cd75
-/* this function called for each hash node during hash destruction */
40cd75
-static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg)
40cd75
+int
40cd75
+memberof_use_txn()
40cd75
 {
40cd75
-	memberof_cached_value *group_ancestor_array;
40cd75
-
40cd75
-	if (he == NULL)
40cd75
-		return HT_ENUMERATE_NEXT;
40cd75
-
40cd75
-
40cd75
-	group_ancestor_array = (memberof_cached_value *) he->value;
40cd75
-	ancestor_hashtable_entry_free(group_ancestor_array);
40cd75
-	slapi_ch_free((void **)&group_ancestor_array);
40cd75
-
40cd75
-	return HT_ENUMERATE_REMOVE;
40cd75
+    return usetxn;
40cd75
 }
40cd75
-
40cd75
-static void ancestor_hashtable_empty(char *msg)
40cd75
-{
40cd75
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
-	long int start;
40cd75
-	struct timespec tsnow;
40cd75
-#endif
40cd75
-
40cd75
-	if (group_ancestors_hashtable) {
40cd75
-		cache_stat.total_enumerate++;
40cd75
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
-		if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
40cd75
-			start = 0;
40cd75
-		} else {
40cd75
-			start = tsnow.tv_nsec;
40cd75
-		}
40cd75
-#endif
40cd75
-		PL_HashTableEnumerateEntries(group_ancestors_hashtable, ancestor_hashtable_remove, msg);
40cd75
-
40cd75
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
40cd75
-		if (start) {
40cd75
-			if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
40cd75
-				cache_stat.cumul_duration_enumerate += (tsnow.tv_nsec - start);
40cd75
-			}
40cd75
-		}
40cd75
-#endif
40cd75
-	}
40cd75
-
40cd75
-}
40cd75
-
40cd75
diff --git a/ldap/servers/plugins/memberof/memberof.h b/ldap/servers/plugins/memberof/memberof.h
40cd75
index 9a3a6a25d..a01c4d247 100644
40cd75
--- a/ldap/servers/plugins/memberof/memberof.h
40cd75
+++ b/ldap/servers/plugins/memberof/memberof.h
40cd75
@@ -62,8 +62,22 @@ typedef struct memberofconfig {
40cd75
 	int skip_nested;
40cd75
 	int fixup_task;
40cd75
 	char *auto_add_oc;
40cd75
+	PLHashTable *ancestors_cache;
40cd75
+	PLHashTable *fixup_cache;
40cd75
 } MemberOfConfig;
40cd75
 
40cd75
+/* The key to access the hash table is the normalized DN
40cd75
+ * The normalized DN is stored in the value because:
40cd75
+ *  - It is used in slapi_valueset_find
40cd75
+ *  - It is used to fill the memberof_get_groups_data.group_norm_vals
40cd75
+ */
40cd75
+typedef struct _memberof_cached_value
40cd75
+{
40cd75
+    char *key;
40cd75
+    char *group_dn_val;
40cd75
+    char *group_ndn_val;
40cd75
+    int valid;
40cd75
+} memberof_cached_value;
40cd75
 
40cd75
 /*
40cd75
  * functions
40cd75
@@ -88,5 +102,8 @@ int memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Ent
40cd75
 void *memberof_get_plugin_id(void);
40cd75
 void memberof_release_config(void);
40cd75
 PRUint64 get_plugin_started(void);
40cd75
+void ancestor_hashtable_entry_free(memberof_cached_value *entry);
40cd75
+PLHashTable *hashtable_new();
40cd75
+int memberof_use_txn();
40cd75
 
40cd75
 #endif	/* _MEMBEROF_H_ */
40cd75
diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c
40cd75
index c3474bf2c..3cc7c4d9c 100644
40cd75
--- a/ldap/servers/plugins/memberof/memberof_config.c
40cd75
+++ b/ldap/servers/plugins/memberof/memberof_config.c
40cd75
@@ -14,12 +14,12 @@
40cd75
  * memberof_config.c - configuration-related code for memberOf plug-in
40cd75
  *
40cd75
  */
40cd75
-
40cd75
+#include "plhash.h"
40cd75
 #include <plstr.h>
40cd75
-
40cd75
 #include "memberof.h"
40cd75
 
40cd75
 #define MEMBEROF_CONFIG_FILTER "(objectclass=*)"
40cd75
+#define MEMBEROF_HASHTABLE_SIZE 1000
40cd75
 
40cd75
 /*
40cd75
  * The configuration attributes are contained in the plugin entry e.g.
40cd75
@@ -33,7 +33,9 @@
40cd75
 
40cd75
 /*
40cd75
  * function prototypes
40cd75
- */ 
40cd75
+ */
40cd75
+static void fixup_hashtable_empty( MemberOfConfig *config, char *msg);
40cd75
+static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg);
40cd75
 static int memberof_validate_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, 
40cd75
 										 int *returncode, char *returntext, void *arg);
40cd75
 static int memberof_search (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, 
40cd75
@@ -48,7 +50,7 @@ static int memberof_search (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_En
40cd75
 /* This is the main configuration which is updated from dse.ldif.  The
40cd75
  * config will be copied when it is used by the plug-in to prevent it
40cd75
  * being changed out from under a running memberOf operation. */
40cd75
-static MemberOfConfig theConfig = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
40cd75
+static MemberOfConfig theConfig = {0};
40cd75
 static Slapi_RWLock *memberof_config_lock = 0;
40cd75
 static int inited = 0;
40cd75
 
40cd75
@@ -696,6 +698,12 @@ memberof_copy_config(MemberOfConfig *dest, MemberOfConfig *src)
40cd75
 {
40cd75
 	if (dest && src)
40cd75
 	{
40cd75
+        /* Allocate our caches here since we only copy the config at the start of an op */
40cd75
+        if (memberof_use_txn() == 1){
40cd75
+            dest->ancestors_cache = hashtable_new();
40cd75
+            dest->fixup_cache = hashtable_new();
40cd75
+        }
40cd75
+
40cd75
 		/* Check if the copy is already up to date */
40cd75
 		if (src->groupattrs)
40cd75
 		{
40cd75
@@ -799,6 +807,14 @@ memberof_free_config(MemberOfConfig *config)
40cd75
 		slapi_ch_free_string(&config->memberof_attr);
40cd75
 		memberof_free_scope(config->entryScopes, &config->entryScopeCount);
40cd75
 		memberof_free_scope(config->entryScopeExcludeSubtrees, &config->entryExcludeScopeCount);
40cd75
+		if (config->fixup_cache) {
40cd75
+			fixup_hashtable_empty(config, "memberof_free_config empty fixup_entry_hastable");
40cd75
+			PL_HashTableDestroy(config->fixup_cache);
40cd75
+		}
40cd75
+		if (config->ancestors_cache) {
40cd75
+			ancestor_hashtable_empty(config, "memberof_free_config empty group_ancestors_hashtable");
40cd75
+			PL_HashTableDestroy(config->ancestors_cache);
40cd75
+		}
40cd75
 	}
40cd75
 }
40cd75
 
40cd75
@@ -1001,3 +1017,131 @@ bail:
40cd75
 
40cd75
 	return ret;
40cd75
 }
40cd75
+
40cd75
+
40cd75
+static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2)
40cd75
+{
40cd75
+	PRIntn rc;
40cd75
+	if (0 == strcasecmp((const char *) v1, (const char *) v2)) {
40cd75
+		rc = 1;
40cd75
+	} else {
40cd75
+		rc = 0;
40cd75
+	}
40cd75
+	return rc;
40cd75
+}
40cd75
+
40cd75
+static PRIntn memberof_hash_compare_values(const void *v1, const void *v2)
40cd75
+{
40cd75
+	PRIntn rc;
40cd75
+	if ((char *) v1 == (char *) v2) {
40cd75
+		rc = 1;
40cd75
+	} else {
40cd75
+		rc = 0;
40cd75
+	}
40cd75
+	return rc;
40cd75
+}
40cd75
+
40cd75
+/*
40cd75
+ *  Hashing function using Bernstein's method
40cd75
+ */
40cd75
+static PLHashNumber memberof_hash_fn(const void *key)
40cd75
+{
40cd75
+    PLHashNumber hash = 5381;
40cd75
+    unsigned char *x = (unsigned char *)key;
40cd75
+    int c;
40cd75
+
40cd75
+    while ((c = *x++)){
40cd75
+        hash = ((hash << 5) + hash) ^ c;
40cd75
+    }
40cd75
+    return hash;
40cd75
+}
40cd75
+
40cd75
+/* allocates the plugin hashtable
40cd75
+ * This hash table is used by operation and is protected from
40cd75
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
40cd75
+ * is not implemented and the hash table will be not used.
40cd75
+ *
40cd75
+ * The hash table contains all the DN of the entries for which the memberof
40cd75
+ * attribute has been computed/updated during the current operation
40cd75
+ *
40cd75
+ * hash table should be empty at the beginning and end of the plugin callback
40cd75
+ */
40cd75
+PLHashTable *hashtable_new(int usetxn)
40cd75
+{
40cd75
+	if (!usetxn) {
40cd75
+		return NULL;
40cd75
+	}
40cd75
+
40cd75
+	return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
40cd75
+		memberof_hash_fn,
40cd75
+		memberof_hash_compare_keys,
40cd75
+		memberof_hash_compare_values, NULL, NULL);
40cd75
+}
40cd75
+
40cd75
+/* this function called for each hash node during hash destruction */
40cd75
+static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
40cd75
+{
40cd75
+	char *dn_copy;
40cd75
+
40cd75
+	if (he == NULL) {
40cd75
+		return HT_ENUMERATE_NEXT;
40cd75
+	}
40cd75
+	dn_copy = (char*) he->value;
40cd75
+	slapi_ch_free_string(&dn_copy);
40cd75
+
40cd75
+	return HT_ENUMERATE_REMOVE;
40cd75
+}
40cd75
+
40cd75
+static void fixup_hashtable_empty(MemberOfConfig *config, char *msg)
40cd75
+{
40cd75
+	if (config->fixup_cache) {
40cd75
+		PL_HashTableEnumerateEntries(config->fixup_cache, fixup_hashtable_remove, msg);
40cd75
+	}
40cd75
+}
40cd75
+
40cd75
+
40cd75
+/* allocates the plugin hashtable
40cd75
+ * This hash table is used by operation and is protected from
40cd75
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
40cd75
+ * is not implemented and the hash table will be not used.
40cd75
+ *
40cd75
+ * The hash table contains all the DN of the entries for which the memberof
40cd75
+ * attribute has been computed/updated during the current operation
40cd75
+ *
40cd75
+ * hash table should be empty at the beginning and end of the plugin callback
40cd75
+ */
40cd75
+
40cd75
+void ancestor_hashtable_entry_free(memberof_cached_value *entry)
40cd75
+{
40cd75
+	int i;
40cd75
+
40cd75
+	for (i = 0; entry[i].valid; i++) {
40cd75
+		slapi_ch_free((void **) &entry[i].group_dn_val);
40cd75
+		slapi_ch_free((void **) &entry[i].group_ndn_val);
40cd75
+	}
40cd75
+	/* Here we are at the ending element containing the key */
40cd75
+	slapi_ch_free((void**) &entry[i].key);
40cd75
+}
40cd75
+
40cd75
+/* this function called for each hash node during hash destruction */
40cd75
+static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
40cd75
+{
40cd75
+    memberof_cached_value *group_ancestor_array;
40cd75
+
40cd75
+    if (he == NULL) {
40cd75
+        return HT_ENUMERATE_NEXT;
40cd75
+    }
40cd75
+    group_ancestor_array = (memberof_cached_value *) he->value;
40cd75
+    ancestor_hashtable_entry_free(group_ancestor_array);
40cd75
+    slapi_ch_free((void **)&group_ancestor_array);
40cd75
+
40cd75
+    return HT_ENUMERATE_REMOVE;
40cd75
+}
40cd75
+
40cd75
+static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg)
40cd75
+{
40cd75
+	if (config->ancestors_cache) {
40cd75
+		PL_HashTableEnumerateEntries(config->ancestors_cache, ancestor_hashtable_remove, msg);
40cd75
+	}
40cd75
+
40cd75
+}
40cd75
-- 
40cd75
2.13.6
40cd75