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

b045b9
From cbe71d7e4901232eaa423b9dc55dba9401c05bec Mon Sep 17 00:00:00 2001
b045b9
From: Mark Reynolds <mreynolds@redhat.com>
b045b9
Date: Fri, 13 Oct 2017 07:09:08 -0400
b045b9
Subject: [PATCH] Ticket 48235 - Remove memberOf global lock
b045b9
b045b9
Bug Description:  The memberOf global lock no longer servers a purpose since
b045b9
                  the plugin is BETXN.  This was causing potential deadlocks
b045b9
                  when multiple backends are used.
b045b9
b045b9
Fix Description:  Remove the lock, and rework the fixup/ancestors caches/hashtables.
b045b9
                  Instead of reusing a single cache, we create a fresh cache
b045b9
                  when we copy the plugin config (which only happens at the start
b045b9
                  of an operation).  Then we destroy the caches when we free
b045b9
                  the config.
b045b9
b045b9
https://pagure.io/389-ds-base/issue/48235
b045b9
b045b9
Reviewed by: firstyear & tbordaz(Thanks!!)
b045b9
b045b9
(cherry picked from commit 184b8a164f4ed456c72d58038aa9a0d512be61fa)
b045b9
---
b045b9
 ldap/servers/plugins/memberof/memberof.c        | 326 +++---------------------
b045b9
 ldap/servers/plugins/memberof/memberof.h        |  17 ++
b045b9
 ldap/servers/plugins/memberof/memberof_config.c | 166 +++++++++++-
b045b9
 3 files changed, 210 insertions(+), 299 deletions(-)
b045b9
b045b9
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
b045b9
index a0f997ddf..a23c52abe 100644
b045b9
--- a/ldap/servers/plugins/memberof/memberof.c
b045b9
+++ b/ldap/servers/plugins/memberof/memberof.c
b045b9
@@ -48,14 +48,11 @@ static Slapi_PluginDesc pdesc = {"memberof", VENDOR,
b045b9
 static void *_PluginID = NULL;
b045b9
 static Slapi_DN *_ConfigAreaDN = NULL;
b045b9
 static Slapi_RWLock *config_rwlock = NULL;
b045b9
-static Slapi_DN *_pluginDN = NULL;
b045b9
-static PRMonitor *memberof_operation_lock = 0;
b045b9
+static Slapi_DN* _pluginDN = NULL;
b045b9
 MemberOfConfig *qsortConfig = 0;
b045b9
 static int usetxn = 0;
b045b9
 static int premodfn = 0;
b045b9
-#define MEMBEROF_HASHTABLE_SIZE 1000
b045b9
-static PLHashTable *fixup_entry_hashtable = NULL;     /* global hash table protected by memberof_lock (memberof_operation_lock) */
b045b9
-static PLHashTable *group_ancestors_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */
b045b9
+
b045b9
 
b045b9
 typedef struct _memberofstringll
b045b9
 {
b045b9
@@ -73,18 +70,6 @@ typedef struct _memberof_get_groups_data
b045b9
     PRBool use_cache;
b045b9
 } memberof_get_groups_data;
b045b9
 
b045b9
-/* The key to access the hash table is the normalized DN
b045b9
- * The normalized DN is stored in the value because:
b045b9
- *  - It is used in slapi_valueset_find
b045b9
- *  - It is used to fill the memberof_get_groups_data.group_norm_vals
b045b9
- */
b045b9
-typedef struct _memberof_cached_value
b045b9
-{
b045b9
-    char *key;
b045b9
-    char *group_dn_val;
b045b9
-    char *group_ndn_val;
b045b9
-    int valid;
b045b9
-} memberof_cached_value;
b045b9
 struct cache_stat
b045b9
 {
b045b9
     int total_lookup;
b045b9
@@ -164,14 +149,9 @@ static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);
b045b9
 static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);
b045b9
 static int memberof_add_objectclass(char *auto_add_oc, const char *dn);
b045b9
 static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc);
b045b9
-static PLHashTable *hashtable_new();
b045b9
-static void fixup_hashtable_empty(char *msg);
b045b9
-static PLHashTable *hashtable_new();
b045b9
-static void ancestor_hashtable_empty(char *msg);
b045b9
-static void ancestor_hashtable_entry_free(memberof_cached_value *entry);
b045b9
-static memberof_cached_value *ancestors_cache_lookup(const char *ndn);
b045b9
-static PRBool ancestors_cache_remove(const char *ndn);
b045b9
-static PLHashEntry *ancestors_cache_add(const void *key, void *value);
b045b9
+static memberof_cached_value *ancestors_cache_lookup(MemberOfConfig *config, const char *ndn);
b045b9
+static PRBool ancestors_cache_remove(MemberOfConfig *config, const char *ndn);
b045b9
+static PLHashEntry *ancestors_cache_add(MemberOfConfig *config, const void *key, void *value);
b045b9
 
b045b9
 /*** implementation ***/
b045b9
 
b045b9
@@ -344,11 +324,6 @@ memberof_postop_start(Slapi_PBlock *pb)
b045b9
     slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
b045b9
                   "--> memberof_postop_start\n");
b045b9
 
b045b9
-    memberof_operation_lock = PR_NewMonitor();
b045b9
-    if (0 == memberof_operation_lock) {
b045b9
-        rc = -1;
b045b9
-        goto bail;
b045b9
-    }
b045b9
     if (config_rwlock == NULL) {
b045b9
         if ((config_rwlock = slapi_new_rwlock()) == NULL) {
b045b9
             rc = -1;
b045b9
@@ -356,9 +331,6 @@ memberof_postop_start(Slapi_PBlock *pb)
b045b9
         }
b045b9
     }
b045b9
 
b045b9
-    fixup_entry_hashtable = hashtable_new();
b045b9
-    group_ancestors_hashtable = hashtable_new();
b045b9
-
b045b9
     /* Set the alternate config area if one is defined. */
b045b9
     slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area);
b045b9
     if (config_area) {
b045b9
@@ -413,13 +385,13 @@ memberof_postop_start(Slapi_PBlock *pb)
b045b9
         goto bail;
b045b9
     }
b045b9
 
b045b9
-/*
b045b9
+    /*
b045b9
      * TODO: start up operation actor thread
b045b9
      * need to get to a point where server failure
b045b9
-         * or shutdown doesn't hose our operations
b045b9
-         * so we should create a task entry that contains
b045b9
+     * or shutdown doesn't hose our operations
b045b9
+     * so we should create a task entry that contains
b045b9
      * all required information to complete the operation
b045b9
-         * then the tasks can be restarted safely if
b045b9
+     * then the tasks can be restarted safely if
b045b9
      * interrupted
b045b9
      */
b045b9
 
b045b9
@@ -451,18 +423,7 @@ memberof_postop_close(Slapi_PBlock *pb __attribute__((unused)))
b045b9
     slapi_sdn_free(&_pluginDN);
b045b9
     slapi_destroy_rwlock(config_rwlock);
b045b9
     config_rwlock = NULL;
b045b9
-    PR_DestroyMonitor(memberof_operation_lock);
b045b9
-    memberof_operation_lock = NULL;
b045b9
-
b045b9
-    if (fixup_entry_hashtable) {
b045b9
-        fixup_hashtable_empty("memberof_postop_close empty fixup_entry_hastable");
b045b9
-        PL_HashTableDestroy(fixup_entry_hashtable);
b045b9
-    }
b045b9
 
b045b9
-    if (group_ancestors_hashtable) {
b045b9
-        ancestor_hashtable_empty("memberof_postop_close empty group_ancestors_hashtable");
b045b9
-        PL_HashTableDestroy(group_ancestors_hashtable);
b045b9
-    }
b045b9
     slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
b045b9
                   "<-- memberof_postop_close\n");
b045b9
     return 0;
b045b9
@@ -524,7 +485,7 @@ memberof_postop_del(Slapi_PBlock *pb)
b045b9
 {
b045b9
     int ret = SLAPI_PLUGIN_SUCCESS;
b045b9
     MemberOfConfig *mainConfig = NULL;
b045b9
-    MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+    MemberOfConfig configCopy = {0};
b045b9
     Slapi_DN *sdn;
b045b9
     void *caller_id = NULL;
b045b9
 
b045b9
@@ -553,9 +514,6 @@ memberof_postop_del(Slapi_PBlock *pb)
b045b9
         memberof_copy_config(&configCopy, memberof_get_config());
b045b9
         memberof_unlock_config();
b045b9
 
b045b9
-        /* get the memberOf operation lock */
b045b9
-        memberof_lock();
b045b9
-
b045b9
         /* remove this DN from the
b045b9
          * membership lists of groups
b045b9
          */
b045b9
@@ -563,7 +521,6 @@ memberof_postop_del(Slapi_PBlock *pb)
b045b9
             slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM,
b045b9
                           "memberof_postop_del - Error deleting dn (%s) from group. Error (%d)\n",
b045b9
                           slapi_sdn_get_dn(sdn), ret);
b045b9
-            memberof_unlock();
b045b9
             goto bail;
b045b9
         }
b045b9
 
b045b9
@@ -583,7 +540,6 @@ memberof_postop_del(Slapi_PBlock *pb)
b045b9
                 }
b045b9
             }
b045b9
         }
b045b9
-        memberof_unlock();
b045b9
     bail:
b045b9
         memberof_free_config(&configCopy);
b045b9
     }
b045b9
@@ -776,7 +732,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn
b045b9
         memberof_cached_value *ht_grp = NULL;
b045b9
         const char *ndn = slapi_sdn_get_ndn(sdn);
b045b9
 
b045b9
-        ht_grp = ancestors_cache_lookup((const void *)ndn);
b045b9
+        ht_grp = ancestors_cache_lookup(config, (const void *)ndn);
b045b9
         if (ht_grp) {
b045b9
 #if MEMBEROF_CACHE_DEBUG
b045b9
             slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn, ht_grp);
b045b9
@@ -918,7 +874,7 @@ memberof_postop_modrdn(Slapi_PBlock *pb)
b045b9
 
b045b9
     if (memberof_oktodo(pb)) {
b045b9
         MemberOfConfig *mainConfig = 0;
b045b9
-        MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+        MemberOfConfig configCopy = {0};
b045b9
         struct slapi_entry *pre_e = NULL;
b045b9
         struct slapi_entry *post_e = NULL;
b045b9
         Slapi_DN *pre_sdn = 0;
b045b9
@@ -944,8 +900,6 @@ memberof_postop_modrdn(Slapi_PBlock *pb)
b045b9
             goto bail;
b045b9
         }
b045b9
 
b045b9
-        memberof_lock();
b045b9
-
b045b9
         /*  update any downstream members */
b045b9
         if (pre_sdn && post_sdn && configCopy.group_filter &&
b045b9
             0 == slapi_filter_test_simple(post_e, configCopy.group_filter)) {
b045b9
@@ -1010,7 +964,6 @@ memberof_postop_modrdn(Slapi_PBlock *pb)
b045b9
                 }
b045b9
             }
b045b9
         }
b045b9
-        memberof_unlock();
b045b9
     bail:
b045b9
         memberof_free_config(&configCopy);
b045b9
     }
b045b9
@@ -1166,7 +1119,7 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
     if (memberof_oktodo(pb)) {
b045b9
         int config_copied = 0;
b045b9
         MemberOfConfig *mainConfig = 0;
b045b9
-        MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+        MemberOfConfig configCopy = {0};
b045b9
 
b045b9
         /* get the mod set */
b045b9
         slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
b045b9
@@ -1209,8 +1162,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
             if (interested) {
b045b9
                 int op = slapi_mod_get_operation(smod);
b045b9
 
b045b9
-                memberof_lock();
b045b9
-
b045b9
                 /* the modify op decides the function */
b045b9
                 switch (op & ~LDAP_MOD_BVALUES) {
b045b9
                 case LDAP_MOD_ADD: {
b045b9
@@ -1221,7 +1172,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
                                       "Error (%d)\n",
b045b9
                                       slapi_sdn_get_dn(sdn), ret);
b045b9
                         slapi_mod_done(next_mod);
b045b9
-                        memberof_unlock();
b045b9
                         goto bail;
b045b9
                     }
b045b9
                     break;
b045b9
@@ -1239,7 +1189,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
                                           "Error (%d)\n",
b045b9
                                           slapi_sdn_get_dn(sdn), ret);
b045b9
                             slapi_mod_done(next_mod);
b045b9
-                            memberof_unlock();
b045b9
                             goto bail;
b045b9
                         }
b045b9
                     } else {
b045b9
@@ -1250,7 +1199,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
                                           "Error (%d)\n",
b045b9
                                           slapi_sdn_get_dn(sdn), ret);
b045b9
                             slapi_mod_done(next_mod);
b045b9
-                            memberof_unlock();
b045b9
                             goto bail;
b045b9
                         }
b045b9
                     }
b045b9
@@ -1265,7 +1213,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
                                       "Error (%d)\n",
b045b9
                                       slapi_sdn_get_dn(sdn), ret);
b045b9
                         slapi_mod_done(next_mod);
b045b9
-                        memberof_unlock();
b045b9
                         goto bail;
b045b9
                     }
b045b9
                     break;
b045b9
@@ -1280,8 +1227,6 @@ memberof_postop_modify(Slapi_PBlock *pb)
b045b9
                     break;
b045b9
                 }
b045b9
                 }
b045b9
-
b045b9
-                memberof_unlock();
b045b9
             }
b045b9
 
b045b9
             slapi_mod_done(next_mod);
b045b9
@@ -1336,7 +1281,7 @@ memberof_postop_add(Slapi_PBlock *pb)
b045b9
 
b045b9
     if (memberof_oktodo(pb) && (sdn = memberof_getsdn(pb))) {
b045b9
         struct slapi_entry *e = NULL;
b045b9
-        MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+        MemberOfConfig configCopy = {0};
b045b9
         MemberOfConfig *mainConfig;
b045b9
         slapi_pblock_get(pb, SLAPI_ENTRY_POST_OP, &e);
b045b9
 
b045b9
@@ -1361,8 +1306,6 @@ memberof_postop_add(Slapi_PBlock *pb)
b045b9
             int i = 0;
b045b9
             Slapi_Attr *attr = 0;
b045b9
 
b045b9
-            memberof_lock();
b045b9
-
b045b9
             for (i = 0; configCopy.groupattrs && configCopy.groupattrs[i]; i++) {
b045b9
                 if (0 == slapi_entry_attr_find(e, configCopy.groupattrs[i], &attr)) {
b045b9
                     if ((ret = memberof_add_attr_list(pb, &configCopy, sdn, attr))) {
b045b9
@@ -1373,8 +1316,6 @@ memberof_postop_add(Slapi_PBlock *pb)
b045b9
                     }
b045b9
                 }
b045b9
             }
b045b9
-
b045b9
-            memberof_unlock();
b045b9
             memberof_free_config(&configCopy);
b045b9
         }
b045b9
     }
b045b9
@@ -2094,7 +2035,7 @@ dump_cache_entry(memberof_cached_value *double_check, const char *msg)
b045b9
  * the firsts elements of the array has 'valid=1' and the dn/ndn of group it belong to
b045b9
  */
b045b9
 static void
b045b9
-cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
b045b9
+cache_ancestors(MemberOfConfig *config, Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
b045b9
 {
b045b9
     Slapi_ValueSet *groupvals = *((memberof_get_groups_data *)groups)->groupvals;
b045b9
     Slapi_Value *sval;
b045b9
@@ -2191,14 +2132,14 @@ cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups)
b045b9
 #if MEMBEROF_CACHE_DEBUG
b045b9
     dump_cache_entry(cache_entry, key);
b045b9
 #endif
b045b9
-    if (ancestors_cache_add((const void *)key_copy, (void *)cache_entry) == NULL) {
b045b9
-        slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Failed to cache ancestor of %s\n", key);
b045b9
+    if (ancestors_cache_add(config, (const void*) key_copy, (void *) cache_entry) == NULL) {
b045b9
+        slapi_log_err( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Failed to cache ancestor of %s\n", key);
b045b9
         ancestor_hashtable_entry_free(cache_entry);
b045b9
-        slapi_ch_free((void **)&cache_entry);
b045b9
+        slapi_ch_free ((void**)&cache_entry);
b045b9
         return;
b045b9
     }
b045b9
 #if MEMBEROF_CACHE_DEBUG
b045b9
-    if (double_check = ancestors_cache_lookup((const void *)key)) {
b045b9
+    if (double_check = ancestors_cache_lookup(config, (const void*) key)) {
b045b9
         dump_cache_entry(double_check, "read back");
b045b9
     }
b045b9
 #endif
b045b9
@@ -2283,8 +2224,7 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn, memberof_get
b045b9
 
b045b9
     merge_ancestors(&member_ndn_val, &member_data, data);
b045b9
     if (!cached && member_data.use_cache)
b045b9
-        cache_ancestors(&member_ndn_val, &member_data);
b045b9
-
b045b9
+        cache_ancestors(config, &member_ndn_val, &member_data);
b045b9
 
b045b9
     slapi_value_free(&member_ndn_val);
b045b9
     slapi_valueset_free(groupvals);
b045b9
@@ -2825,49 +2765,10 @@ memberof_qsort_compare(const void *a, const void *b)
b045b9
                                     val1, val2);
b045b9
 }
b045b9
 
b045b9
-/* betxn: This locking mechanism is necessary to guarantee the memberof
b045b9
- * consistency */
b045b9
-void
b045b9
-memberof_lock()
b045b9
-{
b045b9
-    if (usetxn) {
b045b9
-        PR_EnterMonitor(memberof_operation_lock);
b045b9
-    }
b045b9
-    if (fixup_entry_hashtable) {
b045b9
-        fixup_hashtable_empty("memberof_lock");
b045b9
-    }
b045b9
-    if (group_ancestors_hashtable) {
b045b9
-        ancestor_hashtable_empty("memberof_lock empty group_ancestors_hashtable");
b045b9
-        memset(&cache_stat, 0, sizeof(cache_stat));
b045b9
-    }
b045b9
-}
b045b9
-
b045b9
-void
b045b9
-memberof_unlock()
b045b9
-{
b045b9
-    if (group_ancestors_hashtable) {
b045b9
-        ancestor_hashtable_empty("memberof_unlock empty group_ancestors_hashtable");
b045b9
-#if MEMBEROF_CACHE_DEBUG
b045b9
-        slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics: total lookup %d (success %d), add %d, remove %d, enum %d\n",
b045b9
-                      cache_stat.total_lookup, cache_stat.successfull_lookup,
b045b9
-                      cache_stat.total_add, cache_stat.total_remove, cache_stat.total_enumerate);
b045b9
-        slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics duration: lookup %ld, add %ld, remove %ld, enum %ld\n",
b045b9
-                      cache_stat.cumul_duration_lookup, cache_stat.cumul_duration_add,
b045b9
-                      cache_stat.cumul_duration_remove, cache_stat.cumul_duration_enumerate);
b045b9
-#endif
b045b9
-    }
b045b9
-    if (fixup_entry_hashtable) {
b045b9
-        fixup_hashtable_empty("memberof_lock");
b045b9
-    }
b045b9
-    if (usetxn) {
b045b9
-        PR_ExitMonitor(memberof_operation_lock);
b045b9
-    }
b045b9
-}
b045b9
-
b045b9
 void
b045b9
 memberof_fixup_task_thread(void *arg)
b045b9
 {
b045b9
-    MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+    MemberOfConfig configCopy = {0};
b045b9
     Slapi_Task *task = (Slapi_Task *)arg;
b045b9
     task_data *td = NULL;
b045b9
     int rc = 0;
b045b9
@@ -2933,9 +2834,6 @@ memberof_fixup_task_thread(void *arg)
b045b9
     /* do real work */
b045b9
     rc = memberof_fix_memberof(&configCopy, task, td);
b045b9
 
b045b9
-    /* release the memberOf operation lock */
b045b9
-    memberof_unlock();
b045b9
-
b045b9
 done:
b045b9
     if (usetxn && fixup_pb) {
b045b9
         if (rc) { /* failed */
b045b9
@@ -3100,7 +2998,7 @@ memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *td)
b045b9
 }
b045b9
 
b045b9
 static memberof_cached_value *
b045b9
-ancestors_cache_lookup(const char *ndn)
b045b9
+ancestors_cache_lookup(MemberOfConfig *config, const char *ndn)
b045b9
 {
b045b9
     memberof_cached_value *e;
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
@@ -3118,7 +3016,7 @@ ancestors_cache_lookup(const char *ndn)
b045b9
     }
b045b9
 #endif
b045b9
 
b045b9
-    e = (memberof_cached_value *)PL_HashTableLookupConst(group_ancestors_hashtable, (const void *)ndn);
b045b9
+    e = (memberof_cached_value *) PL_HashTableLookupConst(config->ancestors_cache, (const void *) ndn);
b045b9
 
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
     if (start) {
b045b9
@@ -3133,7 +3031,7 @@ ancestors_cache_lookup(const char *ndn)
b045b9
     return e;
b045b9
 }
b045b9
 static PRBool
b045b9
-ancestors_cache_remove(const char *ndn)
b045b9
+ancestors_cache_remove(MemberOfConfig *config, const char *ndn)
b045b9
 {
b045b9
     PRBool rc;
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
@@ -3151,7 +3049,8 @@ ancestors_cache_remove(const char *ndn)
b045b9
     }
b045b9
 #endif
b045b9
 
b045b9
-    rc = PL_HashTableRemove(group_ancestors_hashtable, (const void *)ndn);
b045b9
+
b045b9
+    rc = PL_HashTableRemove(config->ancestors_cache, (const void *)ndn);
b045b9
 
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
     if (start) {
b045b9
@@ -3164,7 +3063,7 @@ ancestors_cache_remove(const char *ndn)
b045b9
 }
b045b9
 
b045b9
 static PLHashEntry *
b045b9
-ancestors_cache_add(const void *key, void *value)
b045b9
+ancestors_cache_add(MemberOfConfig *config, const void *key, void *value)
b045b9
 {
b045b9
     PLHashEntry *e;
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
@@ -3181,7 +3080,7 @@ ancestors_cache_add(const void *key, void *value)
b045b9
     }
b045b9
 #endif
b045b9
 
b045b9
-    e = PL_HashTableAdd(group_ancestors_hashtable, key, value);
b045b9
+    e = PL_HashTableAdd(config->ancestors_cache, key, value);
b045b9
 
b045b9
 #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
     if (start) {
b045b9
@@ -3211,7 +3110,6 @@ memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
b045b9
     const char *ndn;
b045b9
     char *dn_copy;
b045b9
 
b045b9
-
b045b9
     /*
b045b9
      * If the server is ordered to shutdown, stop the fixup and return an error.
b045b9
      */
b045b9
@@ -3222,7 +3120,7 @@ memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
b045b9
 
b045b9
     /* Check if the entry has not already been fixed */
b045b9
     ndn = slapi_sdn_get_ndn(sdn);
b045b9
-    if (ndn && fixup_entry_hashtable && PL_HashTableLookupConst(fixup_entry_hashtable, (void *)ndn)) {
b045b9
+    if (ndn && config->fixup_cache && PL_HashTableLookupConst(config->fixup_cache, (void *)ndn)) {
b045b9
         slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn);
b045b9
         goto bail;
b045b9
     }
b045b9
@@ -3240,12 +3138,13 @@ memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
b045b9
              * so free this memory
b045b9
              */
b045b9
             ndn = slapi_sdn_get_ndn(sdn);
b045b9
+
b045b9
 #if MEMBEROF_CACHE_DEBUG
b045b9
             slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: This is NOT a group %s\n", ndn);
b045b9
 #endif
b045b9
-            ht_grp = ancestors_cache_lookup((const void *)ndn);
b045b9
+            ht_grp = ancestors_cache_lookup(config, (const void *)ndn);
b045b9
             if (ht_grp) {
b045b9
-                if (ancestors_cache_remove((const void *)ndn)) {
b045b9
+                if (ancestors_cache_remove(config, (const void *)ndn)) {
b045b9
                     slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: free cached values for %s\n", ndn);
b045b9
                     ancestor_hashtable_entry_free(ht_grp);
b045b9
                     slapi_ch_free((void **)&ht_grp);
b045b9
@@ -3297,11 +3196,11 @@ memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
b045b9
     slapi_valueset_free(groups);
b045b9
 
b045b9
     /* records that this entry has been fixed up */
b045b9
-    if (fixup_entry_hashtable) {
b045b9
+    if (config->fixup_cache) {
b045b9
         dn_copy = slapi_ch_strdup(ndn);
b045b9
-        if (PL_HashTableAdd(fixup_entry_hashtable, dn_copy, dn_copy) == NULL) {
b045b9
+        if (PL_HashTableAdd(config->fixup_cache, dn_copy, dn_copy) == NULL) {
b045b9
             slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: "
b045b9
-                                                                      "failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n",
b045b9
+                          "failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n",
b045b9
                           dn_copy, PR_GetError());
b045b9
             slapi_ch_free((void **)&dn_copy);
b045b9
             /* let consider this as not a fatal error, it just skip an optimization */
b045b9
@@ -3397,157 +3296,8 @@ memberof_add_objectclass(char *auto_add_oc, const char *dn)
b045b9
     return rc;
b045b9
 }
b045b9
 
b045b9
-static PRIntn
b045b9
-memberof_hash_compare_keys(const void *v1, const void *v2)
b045b9
-{
b045b9
-    PRIntn rc;
b045b9
-    if (0 == strcasecmp((const char *)v1, (const char *)v2)) {
b045b9
-        rc = 1;
b045b9
-    } else {
b045b9
-        rc = 0;
b045b9
-    }
b045b9
-    return rc;
b045b9
-}
b045b9
-
b045b9
-static PRIntn
b045b9
-memberof_hash_compare_values(const void *v1, const void *v2)
b045b9
-{
b045b9
-    PRIntn rc;
b045b9
-    if ((char *)v1 == (char *)v2) {
b045b9
-        rc = 1;
b045b9
-    } else {
b045b9
-        rc = 0;
b045b9
-    }
b045b9
-    return rc;
b045b9
-}
b045b9
-
b045b9
-/*
b045b9
- *  Hashing function using Bernstein's method
b045b9
- */
b045b9
-static PLHashNumber
b045b9
-memberof_hash_fn(const void *key)
b045b9
-{
b045b9
-    PLHashNumber hash = 5381;
b045b9
-    unsigned char *x = (unsigned char *)key;
b045b9
-    int c;
b045b9
-
b045b9
-    while ((c = *x++)) {
b045b9
-        hash = ((hash << 5) + hash) ^ c;
b045b9
-    }
b045b9
-    return hash;
b045b9
-}
b045b9
-
b045b9
-/* allocates the plugin hashtable
b045b9
- * This hash table is used by operation and is protected from
b045b9
- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
b045b9
- * is not implemented and the hash table will be not used.
b045b9
- *
b045b9
- * The hash table contains all the DN of the entries for which the memberof
b045b9
- * attribute has been computed/updated during the current operation
b045b9
- *
b045b9
- * hash table should be empty at the beginning and end of the plugin callback
b045b9
- */
b045b9
-static PLHashTable *
b045b9
-hashtable_new()
b045b9
-{
b045b9
-    if (!usetxn) {
b045b9
-        return NULL;
b045b9
-    }
b045b9
-
b045b9
-    return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
b045b9
-                           memberof_hash_fn,
b045b9
-                           memberof_hash_compare_keys,
b045b9
-                           memberof_hash_compare_values, NULL, NULL);
b045b9
-}
b045b9
-/* this function called for each hash node during hash destruction */
b045b9
-static PRIntn
b045b9
-fixup_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
b045b9
-{
b045b9
-    char *dn_copy;
b045b9
-
b045b9
-    if (he == NULL) {
b045b9
-        return HT_ENUMERATE_NEXT;
b045b9
-    }
b045b9
-    dn_copy = (char *)he->value;
b045b9
-    slapi_ch_free_string(&dn_copy);
b045b9
-
b045b9
-    return HT_ENUMERATE_REMOVE;
b045b9
-}
b045b9
-
b045b9
-static void
b045b9
-fixup_hashtable_empty(char *msg)
b045b9
-{
b045b9
-    if (fixup_entry_hashtable) {
b045b9
-        PL_HashTableEnumerateEntries(fixup_entry_hashtable, fixup_hashtable_remove, msg);
b045b9
-    }
b045b9
-}
b045b9
-
b045b9
-
b045b9
-/* allocates the plugin hashtable
b045b9
- * This hash table is used by operation and is protected from
b045b9
- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
b045b9
- * is not implemented and the hash table will be not used.
b045b9
- *
b045b9
- * The hash table contains all the DN of the entries for which the memberof
b045b9
- * attribute has been computed/updated during the current operation
b045b9
- *
b045b9
- * hash table should be empty at the beginning and end of the plugin callback
b045b9
- */
b045b9
-
b045b9
-static void
b045b9
-ancestor_hashtable_entry_free(memberof_cached_value *entry)
b045b9
-{
b045b9
-    int i;
b045b9
-    for (i = 0; entry[i].valid; i++) {
b045b9
-        slapi_ch_free((void **)&entry[i].group_dn_val);
b045b9
-        slapi_ch_free((void **)&entry[i].group_ndn_val);
b045b9
-    }
b045b9
-    /* Here we are at the ending element containing the key */
b045b9
-    slapi_ch_free((void **)&entry[i].key);
b045b9
-}
b045b9
-/* this function called for each hash node during hash destruction */
b045b9
-static PRIntn
b045b9
-ancestor_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
b045b9
-{
b045b9
-    memberof_cached_value *group_ancestor_array;
b045b9
-
b045b9
-    if (he == NULL) {
b045b9
-        return HT_ENUMERATE_NEXT;
b045b9
-    }
b045b9
-
b045b9
-
b045b9
-    group_ancestor_array = (memberof_cached_value *)he->value;
b045b9
-    ancestor_hashtable_entry_free(group_ancestor_array);
b045b9
-    slapi_ch_free((void **)&group_ancestor_array);
b045b9
-
b045b9
-    return HT_ENUMERATE_REMOVE;
b045b9
-}
b045b9
-
b045b9
-static void
b045b9
-ancestor_hashtable_empty(char *msg)
b045b9
+int
b045b9
+memberof_use_txn()
b045b9
 {
b045b9
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
-    long int start;
b045b9
-    struct timespec tsnow;
b045b9
-#endif
b045b9
-
b045b9
-    if (group_ancestors_hashtable) {
b045b9
-        cache_stat.total_enumerate++;
b045b9
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
-        if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
b045b9
-            start = 0;
b045b9
-        } else {
b045b9
-            start = tsnow.tv_nsec;
b045b9
-        }
b045b9
-#endif
b045b9
-        PL_HashTableEnumerateEntries(group_ancestors_hashtable, ancestor_hashtable_remove, msg);
b045b9
-
b045b9
-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME)
b045b9
-        if (start) {
b045b9
-            if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
b045b9
-                cache_stat.cumul_duration_enumerate += (tsnow.tv_nsec - start);
b045b9
-            }
b045b9
-        }
b045b9
-#endif
b045b9
-    }
b045b9
+    return usetxn;
b045b9
 }
b045b9
diff --git a/ldap/servers/plugins/memberof/memberof.h b/ldap/servers/plugins/memberof/memberof.h
b045b9
index 4833ce221..ba64e9dfa 100644
b045b9
--- a/ldap/servers/plugins/memberof/memberof.h
b045b9
+++ b/ldap/servers/plugins/memberof/memberof.h
b045b9
@@ -64,8 +64,22 @@ typedef struct memberofconfig
b045b9
     int skip_nested;
b045b9
     int fixup_task;
b045b9
     char *auto_add_oc;
b045b9
+    PLHashTable *ancestors_cache;
b045b9
+    PLHashTable *fixup_cache;
b045b9
 } MemberOfConfig;
b045b9
 
b045b9
+/* The key to access the hash table is the normalized DN
b045b9
+ * The normalized DN is stored in the value because:
b045b9
+ *  - It is used in slapi_valueset_find
b045b9
+ *  - It is used to fill the memberof_get_groups_data.group_norm_vals
b045b9
+ */
b045b9
+typedef struct _memberof_cached_value
b045b9
+{
b045b9
+    char *key;
b045b9
+    char *group_dn_val;
b045b9
+    char *group_ndn_val;
b045b9
+    int valid;
b045b9
+} memberof_cached_value;
b045b9
 
b045b9
 /*
b045b9
  * functions
b045b9
@@ -89,5 +103,8 @@ int memberof_apply_config(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entr
b045b9
 void *memberof_get_plugin_id(void);
b045b9
 void memberof_release_config(void);
b045b9
 PRUint64 get_plugin_started(void);
b045b9
+void ancestor_hashtable_entry_free(memberof_cached_value *entry);
b045b9
+PLHashTable *hashtable_new();
b045b9
+int memberof_use_txn();
b045b9
 
b045b9
 #endif /* _MEMBEROF_H_ */
b045b9
diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c
b045b9
index c5ca4b137..3f22d95d6 100644
b045b9
--- a/ldap/servers/plugins/memberof/memberof_config.c
b045b9
+++ b/ldap/servers/plugins/memberof/memberof_config.c
b045b9
@@ -14,12 +14,12 @@
b045b9
  * memberof_config.c - configuration-related code for memberOf plug-in
b045b9
  *
b045b9
  */
b045b9
-
b045b9
+#include "plhash.h"
b045b9
 #include <plstr.h>
b045b9
-
b045b9
 #include "memberof.h"
b045b9
 
b045b9
 #define MEMBEROF_CONFIG_FILTER "(objectclass=*)"
b045b9
+#define MEMBEROF_HASHTABLE_SIZE 1000
b045b9
 
b045b9
 /*
b045b9
  * The configuration attributes are contained in the plugin entry e.g.
b045b9
@@ -34,14 +34,16 @@
b045b9
 /*
b045b9
  * function prototypes
b045b9
  */
b045b9
-static int memberof_validate_config(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry *e, int *returncode, char *returntext, void *arg);
b045b9
-static int
b045b9
-memberof_search(Slapi_PBlock *pb __attribute__((unused)),
b045b9
-                Slapi_Entry *entryBefore __attribute__((unused)),
b045b9
-                Slapi_Entry *e __attribute__((unused)),
b045b9
-                int *returncode __attribute__((unused)),
b045b9
-                char *returntext __attribute__((unused)),
b045b9
-                void *arg __attribute__((unused)))
b045b9
+static void fixup_hashtable_empty( MemberOfConfig *config, char *msg);
b045b9
+static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg);
b045b9
+static int memberof_validate_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, 
b045b9
+										 int *returncode, char *returntext, void *arg);
b045b9
+static int memberof_search (Slapi_PBlock *pb __attribute__((unused)),
b045b9
+                            Slapi_Entry* entryBefore __attribute__((unused)),
b045b9
+                            Slapi_Entry* e __attribute__((unused)),
b045b9
+                            int *returncode __attribute__((unused)),
b045b9
+                            char *returntext __attribute__((unused)),
b045b9
+                            void *arg __attribute__((unused)))
b045b9
 {
b045b9
     return SLAPI_DSE_CALLBACK_OK;
b045b9
 }
b045b9
@@ -52,7 +54,7 @@ memberof_search(Slapi_PBlock *pb __attribute__((unused)),
b045b9
 /* This is the main configuration which is updated from dse.ldif.  The
b045b9
  * config will be copied when it is used by the plug-in to prevent it
b045b9
  * being changed out from under a running memberOf operation. */
b045b9
-static MemberOfConfig theConfig = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
b045b9
+static MemberOfConfig theConfig = {0};
b045b9
 static Slapi_RWLock *memberof_config_lock = 0;
b045b9
 static int inited = 0;
b045b9
 
b045b9
@@ -693,6 +695,13 @@ void
b045b9
 memberof_copy_config(MemberOfConfig *dest, MemberOfConfig *src)
b045b9
 {
b045b9
     if (dest && src) {
b045b9
+
b045b9
+        /* Allocate our caches here since we only copy the config at the start of an op */
b045b9
+        if (memberof_use_txn() == 1){
b045b9
+            dest->ancestors_cache = hashtable_new();
b045b9
+            dest->fixup_cache = hashtable_new();
b045b9
+        }
b045b9
+
b045b9
         /* Check if the copy is already up to date */
b045b9
         if (src->groupattrs) {
b045b9
             int i = 0, j = 0;
b045b9
@@ -787,6 +796,14 @@ memberof_free_config(MemberOfConfig *config)
b045b9
         slapi_ch_free_string(&config->memberof_attr);
b045b9
         memberof_free_scope(&(config->entryScopes), &config->entryScopeCount);
b045b9
         memberof_free_scope(&(config->entryScopeExcludeSubtrees), &config->entryExcludeScopeCount);
b045b9
+        if (config->fixup_cache) {
b045b9
+            fixup_hashtable_empty(config, "memberof_free_config empty fixup_entry_hastable");
b045b9
+            PL_HashTableDestroy(config->fixup_cache);
b045b9
+        }
b045b9
+        if (config->ancestors_cache) {
b045b9
+            ancestor_hashtable_empty(config, "memberof_free_config empty group_ancestors_hashtable");
b045b9
+            PL_HashTableDestroy(config->ancestors_cache);
b045b9
+        }
b045b9
     }
b045b9
 }
b045b9
 
b045b9
@@ -982,3 +999,130 @@ bail:
b045b9
 
b045b9
     return ret;
b045b9
 }
b045b9
+
b045b9
+
b045b9
+static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2)
b045b9
+{
b045b9
+    PRIntn rc;
b045b9
+    if (0 == strcasecmp((const char *) v1, (const char *) v2)) {
b045b9
+        rc = 1;
b045b9
+    } else {
b045b9
+        rc = 0;
b045b9
+    }
b045b9
+    return rc;
b045b9
+}
b045b9
+
b045b9
+static PRIntn memberof_hash_compare_values(const void *v1, const void *v2)
b045b9
+{
b045b9
+    PRIntn rc;
b045b9
+    if ((char *) v1 == (char *) v2) {
b045b9
+        rc = 1;
b045b9
+    } else {
b045b9
+        rc = 0;
b045b9
+    }
b045b9
+    return rc;
b045b9
+}
b045b9
+
b045b9
+/*
b045b9
+ *  Hashing function using Bernstein's method
b045b9
+ */
b045b9
+static PLHashNumber memberof_hash_fn(const void *key)
b045b9
+{
b045b9
+    PLHashNumber hash = 5381;
b045b9
+    unsigned char *x = (unsigned char *)key;
b045b9
+    int c;
b045b9
+
b045b9
+    while ((c = *x++)){
b045b9
+        hash = ((hash << 5) + hash) ^ c;
b045b9
+    }
b045b9
+    return hash;
b045b9
+}
b045b9
+
b045b9
+/* allocates the plugin hashtable
b045b9
+ * This hash table is used by operation and is protected from
b045b9
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
b045b9
+ * is not implemented and the hash table will be not used.
b045b9
+ *
b045b9
+ * The hash table contains all the DN of the entries for which the memberof
b045b9
+ * attribute has been computed/updated during the current operation
b045b9
+ *
b045b9
+ * hash table should be empty at the beginning and end of the plugin callback
b045b9
+ */
b045b9
+PLHashTable *hashtable_new(int usetxn)
b045b9
+{
b045b9
+    if (!usetxn) {
b045b9
+        return NULL;
b045b9
+    }
b045b9
+
b045b9
+    return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
b045b9
+        memberof_hash_fn,
b045b9
+        memberof_hash_compare_keys,
b045b9
+        memberof_hash_compare_values, NULL, NULL);
b045b9
+}
b045b9
+
b045b9
+/* this function called for each hash node during hash destruction */
b045b9
+static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
b045b9
+{
b045b9
+	char *dn_copy;
b045b9
+
b045b9
+	if (he == NULL) {
b045b9
+		return HT_ENUMERATE_NEXT;
b045b9
+	}
b045b9
+	dn_copy = (char*) he->value;
b045b9
+	slapi_ch_free_string(&dn_copy);
b045b9
+
b045b9
+	return HT_ENUMERATE_REMOVE;
b045b9
+}
b045b9
+
b045b9
+static void fixup_hashtable_empty(MemberOfConfig *config, char *msg)
b045b9
+{
b045b9
+    if (config->fixup_cache) {
b045b9
+        PL_HashTableEnumerateEntries(config->fixup_cache, fixup_hashtable_remove, msg);
b045b9
+    }
b045b9
+}
b045b9
+
b045b9
+
b045b9
+/* allocates the plugin hashtable
b045b9
+ * This hash table is used by operation and is protected from
b045b9
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
b045b9
+ * is not implemented and the hash table will be not used.
b045b9
+ *
b045b9
+ * The hash table contains all the DN of the entries for which the memberof
b045b9
+ * attribute has been computed/updated during the current operation
b045b9
+ *
b045b9
+ * hash table should be empty at the beginning and end of the plugin callback
b045b9
+ */
b045b9
+
b045b9
+void ancestor_hashtable_entry_free(memberof_cached_value *entry)
b045b9
+{
b045b9
+    int i;
b045b9
+
b045b9
+    for (i = 0; entry[i].valid; i++) {
b045b9
+        slapi_ch_free((void **) &entry[i].group_dn_val);
b045b9
+        slapi_ch_free((void **) &entry[i].group_ndn_val);
b045b9
+    }
b045b9
+    /* Here we are at the ending element containing the key */
b045b9
+    slapi_ch_free((void**) &entry[i].key);
b045b9
+}
b045b9
+
b045b9
+/* this function called for each hash node during hash destruction */
b045b9
+static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused)))
b045b9
+{
b045b9
+    memberof_cached_value *group_ancestor_array;
b045b9
+
b045b9
+    if (he == NULL) {
b045b9
+        return HT_ENUMERATE_NEXT;
b045b9
+    }
b045b9
+    group_ancestor_array = (memberof_cached_value *) he->value;
b045b9
+    ancestor_hashtable_entry_free(group_ancestor_array);
b045b9
+    slapi_ch_free((void **)&group_ancestor_array);
b045b9
+
b045b9
+    return HT_ENUMERATE_REMOVE;
b045b9
+}
b045b9
+
b045b9
+static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg)
b045b9
+{
b045b9
+    if (config->ancestors_cache) {
b045b9
+        PL_HashTableEnumerateEntries(config->ancestors_cache, ancestor_hashtable_remove, msg);
b045b9
+    }
b045b9
+}
b045b9
-- 
b045b9
2.13.6
b045b9