From a65b2927980028eba4574f59f9c216ffb0347356 Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Wed, 20 Mar 2013 18:25:41 -0700
Subject: [PATCH 39/42] Ticket #634 - Deadlock in DNA plug-in

Bug description: Fix for Ticket #576 -
DNA: use event queue for config update only at the start up
(commit 7c11ed17796ba8b17afa5758fcfdf1c937b54ef4)
modified the timing of updating the shared config in the
database.  The write operation was no need to be in the DNA
write lock, but it was located in it, which caused the dead-
lock.

Fix description: This patch releases the dna write lock before
updating the shared config in the database.

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

Reviewed by Nathan (Thank you!!)
---
 ldap/servers/plugins/dna/dna.c | 45 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c
index 080e357..d3dfa52 100644
--- a/ldap/servers/plugins/dna/dna.c
+++ b/ldap/servers/plugins/dna/dna.c
@@ -666,6 +666,7 @@ dna_load_plugin_config(int use_eventq)
 
     if (LDAP_SUCCESS != result) {
         status = DNA_FAILURE;
+        dna_unlock();
         goto cleanup;
     }
 
@@ -673,6 +674,7 @@ dna_load_plugin_config(int use_eventq)
                      &entries);
     if (NULL == entries || NULL == entries[0]) {
         status = DNA_SUCCESS;
+        dna_unlock();
         goto cleanup;
     }
 
@@ -682,6 +684,7 @@ dna_load_plugin_config(int use_eventq)
          * looking for valid ones. */
         dna_parse_config_entry(entries[i], 1);
     }
+    dna_unlock();
 
     if (use_eventq) {
         /* Setup an event to update the shared config 30
@@ -692,14 +695,13 @@ dna_load_plugin_config(int use_eventq)
         time(&now);
         slapi_eq_once(dna_update_config_event, NULL, now + 30);
     } else {
-        int nolock = 1; /* no need to lock since dna write lock is held. */
-        dna_update_config_event(0, &nolock);
+        int arg = 0; /* not used. */
+        dna_update_config_event(0, &arg);
     }
 
 cleanup:
     slapi_free_search_results_internal(search_pb);
     slapi_pblock_destroy(search_pb);
-    dna_unlock();
     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
                     "<-- dna_load_plugin_config\n");
 
@@ -1247,15 +1249,8 @@ dna_update_config_event(time_t event_time, void *arg)
     Slapi_PBlock *pb = NULL;
     struct configEntry *config_entry = NULL;
     PRCList *list = NULL;
-    int nolock = 0;
 
-    if (arg) {
-        nolock = *(int *)arg;
-    }
-    if (!nolock) {
-        /* Get read lock to prevent config changes */
-        dna_read_lock();
-    }
+    dna_read_lock();
 
     /* Bail out if the plug-in close function was just called. */
     if (!g_plugin_started) {
@@ -1300,9 +1295,7 @@ dna_update_config_event(time_t event_time, void *arg)
     }
 
 bail:
-    if (!nolock) {
-        dna_unlock();
-    }
+    dna_unlock();
     slapi_pblock_destroy(pb);
 }
 
@@ -2785,7 +2778,6 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
     char **generated_types = NULL;
     PRUint64 setval = 0;
     int i;
-    int issharedconfig = 0;
 
     /* Bail out if the plug-in close function was just called. */
     if (!g_plugin_started) {
@@ -2803,12 +2795,7 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
      *  We also check if we need to get the next range of values, and grab them.
      *  We do this here so we don't have to do it in the be_txn_preop.
      */
-    /* Has write lock for config entries. */
-    issharedconfig = slapi_entry_attr_hasvalue(e, SLAPI_ATTR_OBJECTCLASS,
-                                               DNA_SHAREDCONFIG);
-    if (!issharedconfig) {
-        dna_read_lock();
-    }
+    dna_read_lock();
 
     if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
         list = PR_LIST_HEAD(dna_global_config);
@@ -2966,9 +2953,7 @@ next:
         }
     }
 
-    if (!issharedconfig) {
-        dna_unlock();
-    }
+    dna_unlock();
 
     slapi_ch_array_free(generated_types);
 bail:
@@ -3427,7 +3412,6 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)
     char *type = NULL;
     int numvals, e_numvals = 0;
     int i, len, ret = 0;
-    int issharedconfig = 0;
 
     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
                     "--> dna_be_txn_pre_op\n");
@@ -3463,12 +3447,7 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)
         slapi_mods_init_passin(smods, mods);
     }
 
-    /* Has write lock for config entries. */
-    issharedconfig = slapi_entry_attr_hasvalue(e, SLAPI_ATTR_OBJECTCLASS,
-                                               DNA_SHAREDCONFIG);
-    if (!issharedconfig) {
-        dna_read_lock();
-    }
+    dna_read_lock();
 
     if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
         list = PR_LIST_HEAD(dna_global_config);
@@ -3659,9 +3638,7 @@ next:
             list = PR_NEXT_LINK(list);
         }
     }
-    if (!issharedconfig) {
-        dna_unlock();
-    }
+    dna_unlock();
 bail:
 
     if (LDAP_CHANGETYPE_MODIFY == modtype) {
-- 
1.8.1.4