Ticket 51068 - deadlock when updating the schema

Bug Description:
        It exists a 3 threads deadlock scenario. It involves state change plugins when it
        calls schema_changed_callback. So the trigger is a change of schema (direct or via
        replication). The scenario is
      MOD(cn=schema)    hold StateChange lock   wait for vattr lock
      SRCH              hold vattr lock         wait for DB page
      MOD               hold DB page            wait for StateChange lock

Fix Description:
        Statechange lock protects the list of registered callbacks.
        lock is a mutex where actually registration of callback is only done
        at startup. Later the list is only lookup.
        Making statechange lock a rwlock suppresses the deadlock scenario
        as MODs will only acquire in read StateChange lock.
        It should also improve performance as at the moment all MODs are serialized
        on that lock
	In order to prevent writer starvation a new slapi_new_rwlock_prio
        create rwlock with priority to writers.

https://pagure.io/389-ds-base/issue/51068

Reviewed by: Mark Reynolds, William Brown

Platforms tested: 30

Flag Day: no

Doc impact: no

ldap/servers/plugins/statechange/statechange.c

@@ -40,7 +40,7 @@ static SCNotify *head; /* a place to start in the list */
 #define SCN_PLUGIN_SUBSYSTEM "statechange-plugin" /* used for logging */
 
 static void *api[5];
-static Slapi_Mutex *buffer_lock = 0;
+static Slapi_RWLock *buffer_lock = 0;
 static PRUint64 g_plugin_started = 0;
 
 /*
@@ -139,7 +139,7 @@ statechange_start(Slapi_PBlock *pb __attribute__((unused)))
     api[3] = (void *)_statechange_unregister_all;
     api[4] = (void *)_statechange_vattr_cache_invalidator_callback;
 
-    if (0 == (buffer_lock = slapi_new_mutex())) /* we never free this mutex */
+    if (0 == (buffer_lock = slapi_new_rwlock()))
     {
         /* badness */
         slapi_log_err(SLAPI_LOG_ERR, SCN_PLUGIN_SUBSYSTEM, "statechange_start - Failed to create lock\n");
@@ -179,7 +179,9 @@ statechange_close(Slapi_PBlock *pb __attribute__((unused)))
     slapi_counter_destroy(&op_counter);
 
     slapi_apib_unregister(StateChange_v1_0_GUID);
-    slapi_destroy_mutex(buffer_lock);
+    if (buffer_lock) {
+        slapi_destroy_rwlock(buffer_lock);
+    }
     buffer_lock = NULL;
 
     slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_close\n");
@@ -239,7 +241,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
     slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "--> statechange_post_op\n");
 
     /* evaluate this operation against the notification entries */
-    slapi_lock_mutex(buffer_lock);
+    slapi_rwlock_rdlock(buffer_lock);
     if (head) {
         slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
         if (NULL == sdn) {
@@ -289,7 +291,7 @@ statechange_post_op(Slapi_PBlock *pb, int modtype)
         } while (notify && notify != head);
     }
 bail:
-    slapi_unlock_mutex(buffer_lock);
+    slapi_rwlock_unlock(buffer_lock);
     slapi_log_err(SLAPI_LOG_TRACE, SCN_PLUGIN_SUBSYSTEM, "<-- statechange_post_op\n");
 
     return SLAPI_PLUGIN_SUCCESS; /* always succeed */
@@ -337,7 +339,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
         }
         item->func = func;
 
-        slapi_lock_mutex(buffer_lock);
+        slapi_rwlock_wrlock(buffer_lock);
         if (head == NULL) {
             head = item;
             head->next = head;
@@ -348,7 +350,7 @@ _statechange_register(char *caller_id, char *dn, char *filter, void *caller_data
             head->prev = item;
             item->prev->next = item;
         }
-        slapi_unlock_mutex(buffer_lock);
+        slapi_rwlock_unlock(buffer_lock);
         slapi_ch_free_string(&writable_filter);
 
         ret = SLAPI_PLUGIN_SUCCESS;
@@ -370,7 +372,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
         return ret;
     }
 
-    slapi_lock_mutex(buffer_lock);
+    slapi_rwlock_wrlock(buffer_lock);
 
     if ((func = statechange_find_notify(dn, filter, thefunc))) {
         func->prev->next = func->next;
@@ -391,7 +393,7 @@ _statechange_unregister(char *dn, char *filter, notify_callback thefunc)
         slapi_ch_free((void **)&func);
     }
 
-    slapi_unlock_mutex(buffer_lock);
+    slapi_rwlock_unlock(buffer_lock);
     slapi_counter_decrement(op_counter);
 
     return ret;
@@ -409,7 +411,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
         return;
     }
 
-    slapi_lock_mutex(buffer_lock);
+    slapi_rwlock_wrlock(buffer_lock);
 
     if (notify) {
         do {
@@ -440,7 +442,7 @@ _statechange_unregister_all(char *caller_id, caller_data_free_callback callback)
         } while (notify != start_notify && notify != NULL);
     }
 
-    slapi_unlock_mutex(buffer_lock);
+    slapi_rwlock_unlock(buffer_lock);
     slapi_counter_decrement(op_counter);
 }
 

ldap/servers/slapd/slapi-plugin.h

@@ -6086,6 +6086,22 @@ void slapi_destroy_condvar(Slapi_CondVar *cvar);
 int slapi_wait_condvar(Slapi_CondVar *cvar, struct timeval *timeout);
 int slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all);
 
+/**
+ * Creates a new read/write lock
+ * If prio_writer the rwlock gives priority on writers
+ * else it give priority on readers (default)
+ *
+ * \return A pointer to a \c Slapi_RWLock
+ *
+ * \note Free the returned lock by calling slapi_destroy_rwlock() when finished
+ *
+ * \see slapi_destroy_rwlock()
+ * \see slapi_rwlock_rdlock()
+ * \see slapi_rwlock_wrlock()
+ * \see slapi_rwlock_unlock()
+ */
+Slapi_RWLock *slapi_new_rwlock_prio(int32_t prio_writer);
+
 /**
  * Creates a new read/write lock.
  *

ldap/servers/slapd/slapi2nspr.c

@@ -181,6 +181,31 @@ slapi_notify_condvar(Slapi_CondVar *cvar, int notify_all)
     return (prrc == PR_SUCCESS ? 1 : 0);
 }
 
+Slapi_RWLock *
+slapi_new_rwlock_prio(int32_t prio_writer)
+{
+#ifdef USE_POSIX_RWLOCKS
+    pthread_rwlock_t *rwlock = NULL;
+    pthread_rwlockattr_t attr;
+
+    pthread_rwlockattr_init(&attr);
+    if (prio_writer) {
+        pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP);
+    } else {
+        pthread_rwlockattr_setkind_np(&attr, PTHREAD_RWLOCK_PREFER_READER_NP);
+    }
+
+    rwlock = (pthread_rwlock_t *)slapi_ch_malloc(sizeof(pthread_rwlock_t));
+    if (rwlock) {
+        pthread_rwlock_init(rwlock, &attr);
+    }
+
+    return ((Slapi_RWLock *)rwlock);
+#else
+    return ((Slapi_RWLock *)PR_NewRWLock(PR_RWLOCK_RANK_NONE, "slapi_rwlock"));
+#endif
+}
+
 Slapi_RWLock *
 slapi_new_rwlock(void)
 {

ldap/servers/slapd/vattr.c

@@ -1996,7 +1996,7 @@ vattr_map_create(void)
         return ENOMEM;
     }
 
-    the_map->lock = slapi_new_rwlock();
+    the_map->lock = slapi_new_rwlock_prio(1 /* priority on writers */);
     if (NULL == the_map) {
         slapd_nasty(sourcefile, 3, 0);
         return ENOMEM;