Ticket 47451 - Dynamic Plugin - various fixes

Description:  The previous commit 0e0848a8385463532d53db94c0c8cae912c30eb4 on
              master branch was wrong, and broke the other plugin tasks.

              Upon further testing, some memory leaks and crashes were detected
              and fixed:

              automember.c - Fix memory leak, and PRCList corruption
              dna.c - Fix crash caused by event context not being freed at plugin stop
              memberof.c - pass in the correct function argument, not the pblock
              rootdn_access.c - Fix memory leak
              plugins/uiduniq/uid.c - Add start and close functions to properly handle plugin restarts
              dse.c - Improve plugin restart handling, and properly handle plugin registration
              plugin.c - Properly wait for a plugin to finish before restarting it.
              task.c - Proprely handle the task function arguments

https://fedorahosted.org/389/ticket/47451

Reviewed by: nhosoi(Thanks!)

ldap/servers/plugins/automember/automember.c

@@ -406,7 +406,6 @@ automember_close(Slapi_PBlock * pb)
             automember_task_add_map_entries);
 
     automember_delete_config();
-    slapi_ch_free((void **)&g_automember_config);
     slapi_sdn_free(&_PluginDN);
     slapi_sdn_free(&_ConfigAreaDN);
     slapi_destroy_rwlock(g_automember_config_lock);
@@ -449,6 +448,8 @@ automember_load_config()
     /* Clear out any old config. */
     automember_config_write_lock();
     automember_delete_config();
+    g_automember_config = (PRCList *)slapi_ch_calloc(1, sizeof(struct configEntry));
+    PR_INIT_CLIST(g_automember_config);
 
     search_pb = slapi_pblock_new();
 
@@ -866,6 +867,7 @@ automember_delete_config()
         list = PR_LIST_HEAD(g_automember_config);
         automember_delete_configEntry(list);
     }
+    slapi_ch_free((void **)&g_automember_config);
 
     return;
 }

ldap/servers/plugins/dna/dna.c

@@ -202,6 +202,7 @@ static char *hostname = NULL;
 static char *portnum = NULL;
 static char *secureportnum = NULL;
 
+static Slapi_Eq_Context eq_ctx = {0};
 
 /**
  * server struct for shared ranges
@@ -708,6 +709,7 @@ dna_close(Slapi_PBlock * pb)
     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
                     "--> dna_close\n");
 
+    slapi_eq_cancel(eq_ctx);
     dna_delete_config(NULL);
     slapi_ch_free((void **)&dna_global_config);
     slapi_destroy_rwlock(g_dna_cache_lock);
@@ -869,7 +871,7 @@ dna_load_plugin_config(Slapi_PBlock *pb, int use_eventq)
          * starting up  would cause the change to not
          * get changelogged. */
         time(&now);
-        slapi_eq_once(dna_update_config_event, NULL, now + 30);
+        eq_ctx = slapi_eq_once(dna_update_config_event, NULL, now + 30);
     } else {
         dna_update_config_event(0, NULL);
     }
@@ -2404,7 +2406,7 @@ static int dna_get_next_value(struct configEntry *config_entry,
         } else {
             /* dna_first_free_value() failed for some unknown reason */
             slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
-                            "dna_get_next_value: failed to allocate a new ID!!\n");
+                            "dna_get_next_value: failed to allocate a new ID!! (set(%d) (max: %d)\n",setval,config_entry->maxval);
             goto done;
         }
     }
@@ -3401,7 +3403,7 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
                         ret = dna_first_free_value(config_entry, &setval);
                         if (LDAP_SUCCESS != ret){
                             slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
-                                            "dna_pre_op: failed to allocate a new ID\n");
+                                            "dna_pre_op: failed to allocate a new ID 1\n");
                             /* Set an error string to be returned to the client. */
                             *errstr = slapi_ch_smprintf("Allocation of a new value for range"
                                                " %s failed! Unable to proceed.",
@@ -3412,7 +3414,9 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
                     } else {
                         /* dna_first_free_value() failed for some unknown reason */
                         slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
-                                        "dna_pre_op: failed to allocate a new ID!!\n");
+                                        "dna_pre_op: failed to allocate a new ID!! 2\n");
+                        slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
+                                                    "dna_get_next_value: failed to allocate a new ID!! (set(%d) (max: %d)\n",setval,config_entry->maxval);
                         /* Set an error string to be returned to the client. */
                         *errstr = slapi_ch_smprintf("Allocation of a new value for range"
                                                " %s failed! Unable to proceed.",
@@ -3678,6 +3682,8 @@ _dna_pre_op_modify(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Mods *smods, char **e
                         /* dna_first_free_value() failed for some unknown reason */
                         slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
                                         "dna_pre_op: failed to allocate a new ID!!\n");
+                        slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
+                                                    "dna_get_next_value: failed to allocate a new ID!! (set(%d) (max: %d)\n",setval,config_entry->maxval);
                         /* Set an error string to be returned to the client. */
                         *errstr = slapi_ch_smprintf("Allocation of a new value for range"
                                            " %s failed! Unable to proceed.",

ldap/servers/plugins/memberof/memberof.c

@@ -2740,7 +2740,7 @@ int memberof_task_add(Slapi_PBlock *pb, Slapi_Entry *e,
 	mytaskdata->bind_dn = slapi_ch_strdup(bind_dn);
 
 	/* allocate new task now */
-	task = slapi_plugin_new_task(slapi_entry_get_ndn(e), pb);
+	task = slapi_plugin_new_task(slapi_entry_get_ndn(e), arg);
 
 	/* register our destructor for cleaning up our private data */
 	slapi_task_set_destructor_fn(task, memberof_task_destructor);

ldap/servers/plugins/rootdn_access/rootdn_access.c

@@ -217,7 +217,7 @@ rootdn_close(Slapi_PBlock *pb)
     slapi_ch_array_free(hosts);
     slapi_ch_array_free(hosts_to_deny);
     slapi_ch_array_free(ips);
-    slapi_ch_array_free(ips);
+    slapi_ch_array_free(ips_to_deny);
 
     return 0;
 }
@@ -416,6 +416,8 @@ rootdn_load_config(Slapi_PBlock *pb)
         }
     } else {
         /* failed to get the plugin entry */
+        slapi_log_error(SLAPI_LOG_FATAL, ROOTDN_PLUGIN_SUBSYSTEM, "rootdn_load_config: "
+                "Failed to get plugin entry\n");
         result = -1;
     }
 

ldap/servers/plugins/schema_reload/schema_reload.c

@@ -273,7 +273,7 @@ schemareload_add(Slapi_PBlock *pb, Slapi_Entry *e,
     schemadir = fetch_attr(e, "schemadir", NULL);
 
     /* allocate new task now */
-    task = slapi_plugin_new_task(slapi_entry_get_ndn(e), pb);
+    task = slapi_plugin_new_task(slapi_entry_get_ndn(e), arg);
     if (task == NULL) {
         slapi_log_error(SLAPI_LOG_FATAL, "schemareload", "unable to allocate new task!\n");
         *returncode = LDAP_OPERATIONS_ERROR;

ldap/servers/plugins/uiduniq/uid.c

@@ -1265,6 +1265,38 @@ preop_modrdn(Slapi_PBlock *pb)
 
 }
 
+static int
+uiduniq_start(Slapi_PBlock *pb)
+{
+    Slapi_Entry *plugin_entry = NULL;
+    struct attr_uniqueness_config *config = NULL;
+
+    if (slapi_pblock_get(pb, SLAPI_ADD_ENTRY, &plugin_entry) == 0){
+        /* load the config into the config list */
+        if ((config = uniqueness_entry_to_config(pb, plugin_entry)) == NULL) {
+            return SLAPI_PLUGIN_FAILURE;
+        }
+        slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, (void*) config);
+    }
+
+    return 0;
+}
+
+static int
+uiduniq_close(Slapi_PBlock *pb)
+{
+    Slapi_Entry *plugin_entry = NULL;
+    struct attr_uniqueness_config *config = NULL;
+
+    slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &config);
+    if (config) {
+            slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, NULL);
+            free_uniqueness_config(config);
+            slapi_ch_free((void **) &config);
+    }
+    return 0;
+}
+
 /* ------------------------------------------------------------ */
 /*
  * Initialize the plugin
@@ -1307,13 +1339,6 @@ NSUniqueAttr_Init(Slapi_PBlock *pb)
     }
     slapi_ch_free_string(&plugin_type);
 
-    /* load the config into the config list */
-    if ((config = uniqueness_entry_to_config(pb, plugin_entry)) == NULL) {
-            err = SLAPI_PLUGIN_FAILURE;
-            break;
-    }
-    slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, (void*) config);
-
     /* Provide descriptive information */
     err = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION,
             (void*)&pluginDesc);
@@ -1329,30 +1354,26 @@ NSUniqueAttr_Init(Slapi_PBlock *pb)
     err = slapi_pblock_set(pb, premdn, (void*)preop_modrdn);
     if (err) break;
 
+    err = slapi_pblock_set(pb, SLAPI_PLUGIN_START_FN, (void *) uiduniq_start);
+    if (err) break;
+
+    err = slapi_pblock_set(pb, SLAPI_PLUGIN_CLOSE_FN, (void *) uiduniq_close);
+    if (err) break;
+
+
   END
 
   if (err) {
-    slapi_log_error(SLAPI_LOG_PLUGIN, "NSUniqueAttr_Init",
-             "Error: %d\n", err);
-    if (config) {
-            slapi_pblock_set(pb, SLAPI_PLUGIN_PRIVATE, NULL);
-            free_uniqueness_config(config);
-            slapi_ch_free((void **) &config);
-    }
+    slapi_log_error(SLAPI_LOG_PLUGIN, "NSUniqueAttr_Init", "Error: %d\n", err);
     err = -1;
   }
   else
-    slapi_log_error(SLAPI_LOG_PLUGIN, "NSUniqueAttr_Init",
-             "plugin loaded\n");
+    slapi_log_error(SLAPI_LOG_PLUGIN, "NSUniqueAttr_Init", "plugin loaded\n");
 
   return err;
 }
 
-int
-uidunique_init(Slapi_PBlock *pb)
-{
-  return NSUniqueAttr_Init(pb);
-}
+
 
 
 /* ------------------------------------------------------------ */

ldap/servers/slapd/dse.c

@@ -1827,6 +1827,7 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
     int retval = -1;
     int need_be_postop = 0;
     int plugin_started = 0;
+    int internal_op = 0;
 
     PR_ASSERT(pb);
     if (slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &pdse ) < 0 ||
@@ -1843,6 +1844,8 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
         return retval;
     }
 
+    internal_op = operation_is_flag_set(pb->pb_op, OP_FLAG_INTERNAL);
+
     /* Find the entry we are about to modify. */
     ec = dse_get_entry_copy(pdse, sdn, DSE_USE_LOCK);
     if ( ec == NULL ) {
@@ -2039,20 +2042,22 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
         }
     }
     /*
-     * Perform postop plugin configuration changes
+     * Perform postop plugin configuration changes unless this is an internal operation
      */
-    if(returncode == LDAP_SUCCESS){
-        dse_post_modify_plugin(ec, ecc, mods);
-    } else if(plugin_started){
-        if(plugin_started == 1){
-        	/* the op failed, turn the plugin off */
-            plugin_delete(ecc, returntext, 0 /* not locked */);
-        } else if (plugin_started == 2){
-            /*
-             * This probably can't happen, but...
-             * the op failed, turn the plugin back on.
-             */
-            plugin_add(ecc, returntext, 0 /* not locked */);
+    if(!internal_op){
+        if(returncode == LDAP_SUCCESS){
+            dse_post_modify_plugin(ec, ecc, mods);
+        } else if(plugin_started){
+            if(plugin_started == 1){
+                /* the op failed, turn the plugin off */
+                plugin_delete(ecc, returntext, 0 /* not locked */);
+            } else if (plugin_started == 2){
+                /*
+                 * This probably can't happen, but...
+                 * the op failed, turn the plugin back on.
+                 */
+                plugin_add(ecc, returntext, 0 /* not locked */);
+            }
         }
     }
 
@@ -2065,6 +2070,7 @@ static void
 dse_post_modify_plugin(Slapi_Entry *entryBefore, Slapi_Entry *entryAfter, LDAPMod **mods)
 {
     char *enabled = NULL;
+    int restart_plugin = 1;
     int i;
 
     if (!slapi_entry_attr_hasvalue(entryBefore, SLAPI_ATTR_OBJECTCLASS, "nsSlapdPlugin") ||
@@ -2080,31 +2086,18 @@ dse_post_modify_plugin(Slapi_Entry *entryBefore, Slapi_Entry *entryAfter, LDAPMo
          !strcasecmp(enabled, "on"))
     {
         for(i = 0; mods && mods[i]; i++){
-            /* Check if we are modifying a plugin's config, if so restart the plugin */
-            if (strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_PATH) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_INITFN) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_TYPE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_DEPENDS_ON_TYPE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_DEPENDS_ON_NAMED) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_SCHEMA_CHECK) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_BE_TXN) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_TARGET_SUBTREE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_EXCLUDE_TARGET_SUBTREE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_BIND_SUBTREE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_EXCLUDE_BIND_SUBTREE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_LOAD_NOW) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_LOAD_GLOBAL) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_PRECEDENCE) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_LOG_ACCESS) == 0 ||
-                strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_LOG_AUDIT) == 0 )
-            {
-                /* for all other plugin config changes, restart the plugin */
-                if(plugin_restart(entryBefore, entryAfter) != LDAP_SUCCESS){
-                    slapi_log_error(SLAPI_LOG_FATAL,"dse_post_modify_plugin", "The configuration change "
-                            "for plugin (%s) could not be applied dynamically, and will be ignored until "
-                            "the server is restarted.\n",
-                            slapi_entry_get_dn(entryBefore));
-                }
+            if (strcasecmp(mods[i]->mod_type, ATTR_PLUGIN_ENABLED) == 0){
+                /* we already stop/started the pugin - don't do it again */
+                restart_plugin = 0;
+                break;
+            }
+        }
+        if(restart_plugin){       /* for all other plugin config changes, restart the plugin */
+            if(plugin_restart(entryBefore, entryAfter) != LDAP_SUCCESS){
+                slapi_log_error(SLAPI_LOG_FATAL,"dse_post_modify_plugin", "The configuration change "
+                        "for plugin (%s) could not be applied dynamically, and will be ignored until "
+                        "the server is restarted.\n",
+                        slapi_entry_get_dn(entryBefore));
             }
         }
     }
@@ -2690,7 +2683,10 @@ slapi_config_register_callback_plugin(int operation,
             Slapi_DN dn;
 
             slapi_sdn_init_dn_byref(&dn,base);
-            rc = (NULL != dse_register_callback(pdse, operation, flags, &dn, scope, filter, fn, fn_arg, pb ? pb->pb_plugin: NULL));
+            /* if a pblock was passed, this is a plugin, so set the f_arg as the plugin */
+            rc = (NULL != dse_register_callback(pdse, operation, flags, &dn, scope, filter, fn,
+                                                pb ? (void*)pb->pb_plugin : fn_arg,
+                                                pb ? pb->pb_plugin: NULL));
             slapi_sdn_done(&dn);
         }
     }

ldap/servers/slapd/plugin.c

@@ -58,6 +58,11 @@
 #define CHECK_ALL 0
 #define CHECK_TYPE 1
 
+/* plugin removal flags */
+#define PLUGIN_NOT_FOUND 0
+#define PLUGIN_REMOVED 1
+#define PLUGIN_BUSY 2
+
 static char *critical_plugins[] = { "cn=ldbm database,cn=plugins,cn=config",
                                     "cn=ACL Plugin,cn=plugins,cn=config",
                                     "cn=ACL preoperation,cn=plugins,cn=config",
@@ -3294,7 +3299,7 @@ plugin_remove_plugins(struct slapdplugin *plugin_entry, char *plugin_type)
     struct slapdplugin *plugin = NULL;
     struct slapdplugin *plugin_next = NULL;
     struct slapdplugin *plugin_prev = NULL;
-    int removed = 0;
+    int removed = PLUGIN_NOT_FOUND;
     int type;
 
     /* look everywhere for other plugin functions with the plugin id */
@@ -3315,7 +3320,13 @@ plugin_remove_plugins(struct slapdplugin *plugin_entry, char *plugin_type)
 
                 pblock_init(&pb);
                 plugin_set_stopped(plugin);
-                plugin_op_all_finished(plugin);
+                if (slapi_counter_get_value(plugin->plg_op_counter) > 0){
+                    /*
+                     * Plugin is still busy, and we might be blocking it
+                     * by holding global plugin lock so return for now.
+                     */
+                    return PLUGIN_BUSY;
+                }
                 plugin_call_one( plugin, SLAPI_PLUGIN_CLOSE_FN, &pb);
 
                 if(plugin_prev){
@@ -3332,7 +3343,7 @@ plugin_remove_plugins(struct slapdplugin *plugin_entry, char *plugin_type)
                 plugin_remove_from_shutdown(plugin);
                 plugin->plg_removed = 1;
                 plugin->plg_started = 0;
-                removed = 1;
+                removed = PLUGIN_REMOVED;
             } else {
                 plugin_prev = plugin;
             }
@@ -3362,15 +3373,10 @@ plugin_delete(Slapi_Entry *plugin_entry, char *returntext, int locked)
     const char *plugin_dn = slapi_entry_get_dn_const(plugin_entry);
     char *value = NULL;
     int td_locked = 1;
-    int removed = 0;
+    int removed = PLUGIN_BUSY;
     int type = 0;
     int rc = LDAP_SUCCESS;
 
-    if(!locked){
-        slapi_rwlock_wrlock(global_rwlock);
-    }
-    slapi_td_set_plugin_locked(&td_locked);
-
     /* Critical server plugins can not be disabled */
     if(plugin_is_critical(plugin_entry)){
         LDAPDebug(LDAP_DEBUG_ANY, "plugin_delete: plugin \"%s\" is critical to server operations, and can not be disabled\n",
@@ -3390,54 +3396,62 @@ plugin_delete(Slapi_Entry *plugin_entry, char *returntext, int locked)
         rc = -1;
         goto done;
     } else {
-        rc = plugin_get_type_and_list(value, &type, &plugin_list);
-        if ( rc != 0 ) {
-            /* error: unknown plugin type */
-            LDAPDebug(LDAP_DEBUG_ANY, "plugin_delete: unknown plugin type \"%s\" in entry \"%s\"\n",
-                      value, slapi_entry_get_dn_const(plugin_entry), 0);
-            PR_snprintf (returntext, SLAPI_DSE_RETURNTEXT_SIZE, "Plugin delete failed: unknown plugin type "
+        while(removed == PLUGIN_BUSY){
+            removed = PLUGIN_NOT_FOUND;
+            if(!locked){
+                slapi_rwlock_wrlock(global_rwlock);
+            }
+            slapi_td_set_plugin_locked(&td_locked);
+
+            rc = plugin_get_type_and_list(value, &type, &plugin_list);
+            if ( rc != 0 ) {
+                /* error: unknown plugin type */
+                LDAPDebug(LDAP_DEBUG_ANY, "plugin_delete: unknown plugin type \"%s\" in entry \"%s\"\n",
+                          value, slapi_entry_get_dn_const(plugin_entry), 0);
+                PR_snprintf (returntext, SLAPI_DSE_RETURNTEXT_SIZE, "Plugin delete failed: unknown plugin type "
                          "\"%s\" in entry.",value);
-            rc = -1;
-            goto done;
-        }
+                rc = -1;
+                goto unlock;
+            }
 
-        /*
-         * Skip syntax/matching rule/database plugins - these can not be disabled as it
-         * could break existing schema.  We allow the update to occur, but it will
-         * not take effect until the next server restart.
-         */
-        if(type == SLAPI_PLUGIN_SYNTAX || type == SLAPI_PLUGIN_MATCHINGRULE || type == SLAPI_PLUGIN_DATABASE){
-            removed = 1;  /* avoids error check below */
-            goto done;
-        }
+            /*
+             * Skip syntax/matching rule/database plugins - these can not be disabled as it
+             * could break existing schema.  We allow the update to occur, but it will
+             * not take effect until the next server restart.
+             */
+            if(type == SLAPI_PLUGIN_SYNTAX || type == SLAPI_PLUGIN_MATCHINGRULE || type == SLAPI_PLUGIN_DATABASE){
+                removed = PLUGIN_REMOVED;  /* avoids error check below */
+                goto unlock;
+            }
 
-        /*
-         * Now remove the plugin from the list and the hashtable
-         */
-        for(plugin = *plugin_list; plugin ; plugin = plugin->plg_next){
-            if(strcasecmp(plugin->plg_dn, plugin_dn) == 0){
-                /*
-                 * Make sure there are no other plugins that depend on this one before removing it
-                 */
-                if(plugin_delete_check_dependency(plugin, CHECK_ALL, returntext) != LDAP_SUCCESS){
-                    LDAPDebug(LDAP_DEBUG_ANY, "plugin_delete: failed to disable/delete plugin (%s)\n",
-                              plugin->plg_dn,0,0);
-                    rc = -1;
-                    goto done;
+            /*
+             * Now remove the plugin from the list and the hashtable
+             */
+            for(plugin = *plugin_list; plugin ; plugin = plugin->plg_next){
+                if(strcasecmp(plugin->plg_dn, plugin_dn) == 0){
+                    /*
+                     * Make sure there are no other plugins that depend on this one before removing it
+                     */
+                    if(plugin_delete_check_dependency(plugin, CHECK_ALL, returntext) != LDAP_SUCCESS){
+                        LDAPDebug(LDAP_DEBUG_ANY, "plugin_delete: failed to disable/delete plugin (%s)\n",
+                                  plugin->plg_dn,0,0);
+                        rc = -1;
+                        break;
+                    }
+                    removed = plugin_remove_plugins(plugin, value);
+                    break;
                 }
-                removed = plugin_remove_plugins(plugin, value);
-                break;
             }
+unlock:
+            if(!locked){
+                slapi_rwlock_unlock(global_rwlock);
+            }
+            td_locked = 0;
+            slapi_td_set_plugin_locked(&td_locked);
         }
     }
 
 done:
-    if(!locked){
-        slapi_rwlock_unlock(global_rwlock);
-    }
-    td_locked = 0;
-    slapi_td_set_plugin_locked(&td_locked);
-
     slapi_ch_free_string(&value);
 
     if(!removed && rc == 0){

ldap/servers/slapd/task.c

@@ -131,9 +131,9 @@ slapi_new_task(const char *dn)
 }
 
 Slapi_Task *
-slapi_plugin_new_task(const char *dn, void *plugin_pb)
+slapi_plugin_new_task(const char *dn, void *plugin)
 {
-    return new_task(dn, plugin_pb);
+    return new_task(dn, plugin);
 }
 
 /* slapi_destroy_task: destroy a task
@@ -540,7 +540,7 @@ slapi_plugin_task_register_handler(const char *name, dseCallbackFn func, Slapi_P
 
     /* register add callback */
     slapi_config_register_callback_plugin(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP,
-                                   dn, LDAP_SCOPE_SUBTREE, "(objectclass=*)", func, NULL, plugin_pb);
+                                   dn, LDAP_SCOPE_SUBTREE, "(objectclass=*)", func, plugin_pb, plugin_pb);
     /* deny modify/delete of the root task entry */
     slapi_config_register_callback(SLAPI_OPERATION_MODIFY, DSE_FLAG_PREOP,
                                    dn, LDAP_SCOPE_BASE, "(objectclass=*)", task_deny, NULL);
@@ -583,11 +583,9 @@ void slapi_task_set_cancel_fn(Slapi_Task *task, TaskCallbackFn func)
  ***********************************/
 /* create a new task, fill in DN, and setup modify callback */
 static Slapi_Task *
-new_task(const char *rawdn, void *plugin_pb)
+new_task(const char *rawdn, void *plugin)
 {
     Slapi_Task *task = NULL;
-    Slapi_PBlock *pb = (Slapi_PBlock *)plugin_pb;
-    void *plugin = pb ? pb->pb_plugin : NULL;
     char *dn = NULL;
 
     if (rawdn == NULL) {