From 78b344ff88f488aa63ccccf464e83ad55ee9e96d Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Tue, 16 Dec 2014 15:25:35 -0500

Subject: [PATCH 50/53] 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!)

(cherry picked from commit ff023a48b7fc78711eed8ae11e67057597acdeb3)

(cherry picked from commit f17159e73ba0851599c4e50c1bf52d3d10f2711f)

---

 ldap/servers/plugins/automember/automember.c       |   4 +-

 ldap/servers/plugins/dna/dna.c                     |  14 ++-

 ldap/servers/plugins/memberof/memberof.c           |   2 +-

 ldap/servers/plugins/rootdn_access/rootdn_access.c |   4 +-

 ldap/servers/plugins/schema_reload/schema_reload.c |   2 +-

 ldap/servers/plugins/uiduniq/uid.c                 |  63 ++++++++----

 ldap/servers/slapd/dse.c                           |  74 +++++++-------

 ldap/servers/slapd/plugin.c                        | 110 ++++++++++++---------

 ldap/servers/slapd/task.c                          |  10 +-

 9 files changed, 161 insertions(+), 122 deletions(-)

diff --git a/ldap/servers/plugins/automember/automember.c b/ldap/servers/plugins/automember/automember.c

index c443a65..6a8fd22 100644

--- a/ldap/servers/plugins/automember/automember.c

+++ b/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;

 }

diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c

index 75edca8..ded0bbb 100644

--- a/ldap/servers/plugins/dna/dna.c

+++ b/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.",

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index cb4ef75..bfa733a 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/ldap/servers/plugins/memberof/memberof.c

@@ -2764,7 +2764,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);

diff --git a/ldap/servers/plugins/rootdn_access/rootdn_access.c b/ldap/servers/plugins/rootdn_access/rootdn_access.c

index 9122f9b..3045e9f 100644

--- a/ldap/servers/plugins/rootdn_access/rootdn_access.c

+++ b/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;

     }

diff --git a/ldap/servers/plugins/schema_reload/schema_reload.c b/ldap/servers/plugins/schema_reload/schema_reload.c

index 3ff4c4d..b1a5bb8 100644

--- a/ldap/servers/plugins/schema_reload/schema_reload.c

+++ b/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;

diff --git a/ldap/servers/plugins/uiduniq/uid.c b/ldap/servers/plugins/uiduniq/uid.c

index 07ff0ec..f37ab8c 100644

--- a/ldap/servers/plugins/uiduniq/uid.c

+++ b/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);

-}

+

 /* ------------------------------------------------------------ */

diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c

index fc5f492..f0ce255 100644

--- a/ldap/servers/slapd/dse.c

+++ b/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;;

         }

     }

diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c

index dde3ce5..820c25f 100644

--- a/ldap/servers/slapd/plugin.c

+++ b/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){

diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c

index b1f7652..98ec88c 100644

--- a/ldap/servers/slapd/task.c

+++ b/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) {

-- 

1.9.3