[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fedora-directory-commits
Subject:    [389-commits] Branch '389-ds-base-1.2.11' - ldap/servers
From:       Noriko Hosoi <nhosoi () fedoraproject ! org>
Date:       2013-03-25 23:03:50
Message-ID: 20130325230350.B38A112A1 () fedorahosted ! org
[Download RAW message or body]

 ldap/servers/plugins/dna/dna.c |   45 ++++++++++-------------------------------
 1 file changed, 11 insertions(+), 34 deletions(-)

New commits:
commit 8bd3a0db0eee06130575bded6938480cc6509dc0
Author: Noriko Hosoi <nhosoi@redhat.com>
Date:   Wed Mar 20 18:25:41 2013 -0700

    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!!)

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) {


--
389 commits mailing list
389-commits@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-commits
[prev in list] [next in list] [prev in thread] [next in thread]