Browse Source

Ticket 47936: Create a global lock to serialize write operations over several backends

Bug Description:
	Some txn-post plugin may trigger operation on other backend.
	This can easily conduct to deadlock, with locks (backend locks, plugin locks
	or db page lock) taken in opposite order.

Fix Description:
        Creating a global lock that will be acquired when updating any backend
	(any ldbm database, cn=config, cn=schema).
	It introduces a new config attribute:
	dn: cn=config
	nsslapd-global-backend-lock: <on|off(default)>

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

Reviewed by: Rich, Ludwig, Noriko (thanks to all of you !)

Platforms tested: F20/F21

Flag Day: no

Doc impact: no
Thierry bordaz (tbordaz) 10 years ago
parent
commit
8583012c75

+ 7 - 0
ldap/servers/slapd/back-ldbm/dblayer.c

@@ -3968,6 +3968,9 @@ void dblayer_lock_backend(backend *be)
     ldbm_instance *inst;
 
     PR_ASSERT(NULL != be);
+    if (global_backend_lock_requested()) {
+        global_backend_lock_lock();
+    }
     inst = (ldbm_instance *) be->be_instance_info;
     PR_ASSERT(NULL != inst);
     
@@ -3987,6 +3990,10 @@ void dblayer_unlock_backend(backend *be)
     if (NULL != inst->inst_db_mutex) {
         PR_ExitMonitor(inst->inst_db_mutex);
     }
+    
+    if (global_backend_lock_requested()) {
+        global_backend_lock_unlock();
+    }
 }
 
 

+ 29 - 0
ldap/servers/slapd/backend.c

@@ -43,6 +43,9 @@
 /* backend.c - Slapi_Backend methods */
 
 #include "slap.h"
+#include "nspr.h"
+
+static PRMonitor *global_backend_mutex = NULL;
 
 void
 be_init( Slapi_Backend *be, const char *type, const char *name, int isprivate, int logchanges, int sizelimit, int timelimit )
@@ -147,6 +150,32 @@ be_done(Slapi_Backend *be)
     }
 }
 
+void
+global_backend_lock_init()
+{
+    global_backend_mutex = PR_NewMonitor();
+}
+
+int
+global_backend_lock_requested()
+{
+    return config_get_global_backend_lock();
+}
+void
+global_backend_lock_lock() 
+{
+    if (global_backend_mutex) {
+        PR_EnterMonitor(global_backend_mutex);
+    }
+}
+
+void
+global_backend_lock_unlock() {
+    if (global_backend_mutex) {
+        PR_ExitMonitor(global_backend_mutex);
+    }
+}
+
 void
 slapi_be_delete_onexit (Slapi_Backend *be)
 {

+ 30 - 2
ldap/servers/slapd/dse.c

@@ -1828,6 +1828,7 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
     int need_be_postop = 0;
     int plugin_started = 0;
     int internal_op = 0;
+    PRBool global_lock_owned = PR_FALSE;
 
     PR_ASSERT(pb);
     if (slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &pdse ) < 0 ||
@@ -1871,6 +1872,12 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
     /* Modify a copy of the entry*/
     ecc = slapi_entry_dup( ec );
     err = entry_apply_mods( ecc, mods );
+    
+    /* Possibly acquire the global backend lock */
+    if (global_backend_lock_requested()) {
+        global_backend_lock_lock();
+        global_lock_owned = PR_TRUE;
+    }
 
     /* XXXmcs: should we expand objectclass values here?? */
     /* give the dse callbacks the first crack at the modify */
@@ -2069,7 +2076,9 @@ dse_modify(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
             }
         }
     }
-
+    if (global_lock_owned) {
+        global_backend_lock_unlock();
+    }
     slapi_send_ldap_result( pb, returncode, NULL, returntext[0] ? returntext : NULL, 0, NULL );
 
     return dse_modify_return(retval, ec, ecc);
@@ -2224,6 +2233,7 @@ dse_add(Slapi_PBlock *pb) /* JCM There should only be one exit point from this f
     Slapi_DN *sdn = NULL;
     Slapi_DN parent;
     int need_be_postop = 0;
+    PRBool global_lock_owned = PR_FALSE;
 
     /*
      * Get the database, the dn and the entry to add
@@ -2345,6 +2355,12 @@ dse_add(Slapi_PBlock *pb) /* JCM There should only be one exit point from this f
         goto done;
     }
 
+    /* Possibly acquire the global backend lock */
+    if (global_backend_lock_requested()) {
+        global_backend_lock_lock();
+        global_lock_owned = PR_TRUE;
+    }
+
     if(dse_call_callback(pdse, pb, SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, e,
                          NULL, &returncode, returntext)!=SLAPI_DSE_CALLBACK_OK) {
         if (!returncode) {
@@ -2424,7 +2440,9 @@ dse_add(Slapi_PBlock *pb) /* JCM There should only be one exit point from this f
             slapi_pblock_get(pb, SLAPI_RESULT_CODE, &returncode);
         }
     }
-
+    if (global_lock_owned) {
+        global_backend_lock_unlock();
+    }
     slapi_send_ldap_result(pb, returncode, NULL, returntext[0] ? returntext : NULL, 0, NULL );
     return dse_add_return(rc, e);
 }
@@ -2456,6 +2474,7 @@ dse_delete(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
     Slapi_Entry *ec = NULL; /* copy of entry to delete */
     Slapi_Entry *orig_entry = NULL;
     int need_be_postop = 0;
+    PRBool global_lock_owned = PR_FALSE;
 
     /*
      * Get the database and the dn
@@ -2500,6 +2519,12 @@ dse_delete(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi
         goto done;
     }
 
+    /* Possibly acquire the global backend lock */
+    if (global_backend_lock_requested()) {
+        global_backend_lock_lock();
+        global_lock_owned = PR_TRUE;
+    }
+
     if(dse_call_callback(pdse, pb, SLAPI_OPERATION_DELETE, DSE_FLAG_PREOP, ec, NULL, &returncode,returntext)==SLAPI_DSE_CALLBACK_OK) {
         slapi_pblock_set(pb, SLAPI_DELETE_BEPREOP_ENTRY, ec);
         slapi_pblock_set(pb, SLAPI_RESULT_CODE, &returncode);
@@ -2552,6 +2577,9 @@ done:
             slapi_pblock_get(pb, SLAPI_RESULT_CODE, &returncode);
         }
     }
+    if (global_lock_owned) {
+        global_backend_lock_unlock();
+    }
     if (returncode && !returntext[0]) {
         char *ldap_result_message = NULL;
         slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);

+ 32 - 1
ldap/servers/slapd/libglobs.c

@@ -273,6 +273,7 @@ slapi_int_t init_listen_backlog_size;
 slapi_onoff_t init_ignore_time_skew;
 slapi_onoff_t init_dynamic_plugins;
 slapi_onoff_t init_cn_uses_dn_syntax_in_dns;
+slapi_onoff_t init_global_backend_local;
 #if defined (LINUX)
 slapi_int_t init_malloc_mxfast;
 slapi_int_t init_malloc_trim_threshold;
@@ -1122,7 +1123,11 @@ static struct config_get_and_set {
 	{CONFIG_IGNORE_TIME_SKEW, config_set_ignore_time_skew,
 		NULL, 0,
 		(void**)&global_slapdFrontendConfig.ignore_time_skew,
-		CONFIG_ON_OFF, (ConfigGetFunc)config_get_ignore_time_skew, &init_ignore_time_skew}
+		CONFIG_ON_OFF, (ConfigGetFunc)config_get_ignore_time_skew, &init_ignore_time_skew},
+	{CONFIG_GLOBAL_BACKEND_LOCK, config_set_global_backend_lock,
+		NULL, 0,
+		(void**)&global_slapdFrontendConfig.global_backend_lock,
+		CONFIG_ON_OFF, (ConfigGetFunc)config_get_global_backend_lock, &init_global_backend_local}	
 #ifdef MEMPOOL_EXPERIMENTAL
 	,{CONFIG_MEMPOOL_SWITCH_ATTRIBUTE, config_set_mempool_switch,
 		NULL, 0,
@@ -1570,6 +1575,7 @@ FrontendConfig_init () {
   init_ignore_time_skew = cfg->ignore_time_skew = LDAP_OFF;
   init_dynamic_plugins = cfg->dynamic_plugins = LDAP_OFF;
   init_cn_uses_dn_syntax_in_dns = cfg->cn_uses_dn_syntax_in_dns = LDAP_OFF;
+  init_global_backend_local = LDAP_OFF;
 #if defined(LINUX)
   init_malloc_mxfast = cfg->malloc_mxfast = DEFAULT_MALLOC_UNSET;
   init_malloc_trim_threshold = cfg->malloc_trim_threshold = DEFAULT_MALLOC_UNSET;
@@ -7244,6 +7250,18 @@ config_get_ignore_time_skew(void)
     return retVal;
 }
 
+int
+config_get_global_backend_lock()
+{
+    int retVal;
+    slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
+    CFG_LOCK_READ(slapdFrontendConfig);
+    retVal = slapdFrontendConfig->global_backend_lock;
+    CFG_UNLOCK_READ(slapdFrontendConfig);
+
+    return retVal;
+}
+
 int
 config_set_enable_turbo_mode( const char *attrname, char *value,
                             char *errorbuf, int apply )
@@ -7283,6 +7301,19 @@ config_set_ignore_time_skew( const char *attrname, char *value,
     return retVal;
 }
 
+int
+config_set_global_backend_lock( const char *attrname, char *value,
+	                        char *errorbuf, int apply )
+{
+    int retVal = LDAP_SUCCESS;
+    slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig();
+
+    retVal = config_set_onoff(attrname, value,
+                              &(slapdFrontendConfig->global_backend_lock),
+                              errorbuf, apply);
+    return retVal;
+}
+
 int
 config_set_plugin_logging( const char *attrname, char *value,
                             char *errorbuf, int apply )

+ 1 - 0
ldap/servers/slapd/main.c

@@ -1057,6 +1057,7 @@ main( int argc, char **argv)
 	/* initialize the normalized DN cache */
 	ndn_cache_init();
 
+	global_backend_lock_init();
 	/*
 	 * Detach ourselves from the terminal (unless running in debug mode).
 	 * We must detach before we start any threads since detach forks() on

+ 6 - 0
ldap/servers/slapd/proto-slap.h

@@ -213,6 +213,10 @@ void be_addsuffix(Slapi_Backend *be,const Slapi_DN *suffix);
 Slapi_DN *be_getconfigdn(Slapi_Backend *be,Slapi_DN *dn);
 Slapi_DN *be_getmonitordn(Slapi_Backend *be,Slapi_DN *dn);
 int be_writeconfig (Slapi_Backend *be);
+void global_backend_lock_init();
+int global_backend_lock_requested();
+void global_backend_lock_lock();
+void global_backend_lock_unlock();
 
 /*
  * backend_manager.c
@@ -404,6 +408,7 @@ int config_set_return_orig_type_switch(const char *attrname, char *value, char *
 int config_set_sasl_maxbufsize(const char *attrname, char *value, char *errorbuf, int apply );
 int config_set_listen_backlog_size(const char *attrname, char *value, char *errorbuf, int apply);
 int config_set_ignore_time_skew(const char *attrname, char *value, char *errorbuf, int apply);
+int config_set_global_backend_lock(const char *attrname, char *value, char *errorbuf, int apply);
 #if defined(LINUX)
 int config_set_malloc_mxfast(const char *attrname, char *value, char *errorbuf, int apply);
 int config_set_malloc_trim_threshold(const char *attrname, char *value, char *errorbuf, int apply);
@@ -596,6 +601,7 @@ int config_get_cn_uses_dn_syntax_in_dns();
 PLHashNumber hashNocaseString(const void *key);
 PRIntn hashNocaseCompare(const void *v1, const void *v2);
 int config_get_ignore_time_skew();
+int config_get_global_backend_lock();
 
 #if defined(LINUX)
 int config_get_malloc_mxfast();

+ 2 - 0
ldap/servers/slapd/slap.h

@@ -2117,6 +2117,7 @@ typedef struct _slapdEntryPoints {
 #define CONFIG_REWRITE_RFC1274_ATTRIBUTE "nsslapd-rewrite-rfc1274"
 #define CONFIG_PLUGIN_BINDDN_TRACKING_ATTRIBUTE "nsslapd-plugin-binddn-tracking"
 #define CONFIG_MODDN_ACI_ATTRIBUTE "nsslapd-moddn-aci"
+#define CONFIG_GLOBAL_BACKEND_LOCK "nsslapd-global-backend-lock"
 
 #define CONFIG_CONFIG_ATTRIBUTE "nsslapd-config"
 #define CONFIG_INSTDIR_ATTRIBUTE "nsslapd-instancedir"
@@ -2423,6 +2424,7 @@ typedef struct _slapdFrontendConfig {
   slapi_onoff_t ignore_time_skew;
   slapi_onoff_t dynamic_plugins; /* allow plugins to be dynamically enabled/disabled */
   slapi_onoff_t cn_uses_dn_syntax_in_dns; /* indicates the cn value in dns has dn syntax */
+  slapi_onoff_t global_backend_lock;
 #if defined(LINUX)
   int malloc_mxfast;            /* mallopt M_MXFAST */
   int malloc_trim_threshold;    /* mallopt M_TRIM_THRESHOLD */