andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone

Blame SOURCES/0017-Issue-50538-cleanAllRUV-task-limit-is-not-enforced-f.patch

232633
From bc438614f71d18e337a56b49a67627299658b649 Mon Sep 17 00:00:00 2001
232633
From: Mark Reynolds <mreynolds@redhat.com>
232633
Date: Wed, 7 Aug 2019 20:36:53 -0400
232633
Subject: [PATCH 1/4] Issue 50538 - cleanAllRUV task limit is not enforced for
232633
 replicated tasks
232633
232633
Bug Description:
232633
232633
There is a hard limit of 64 concurrent cleanAllRUV tasks, but this limit is
232633
only enforced when creating "new" tasks. It was not enforced when a task was
232633
received via an extended operation. There were also race conditions in the
232633
existing logic that allowed the array of cleaned rids to get corrupted . This
232633
allowed for a very large number of task threads to be created.
232633
232633
Fix Description:
232633
232633
Maintain a new counter to keep track of the number of clean and abort threads
232633
to make sure it never over runs the rid array buffers.
232633
232633
relates: https://pagure.io/389-ds-base/issue/50538
232633
232633
Reviewed by: lkrispenz(Thanks!)
232633
---
232633
 .../suites/replication/cleanallruv_test.py    | 144 +++++++++-
232633
 ldap/servers/plugins/replication/repl5.h      |   7 +-
232633
 .../replication/repl5_replica_config.c        | 247 ++++++++++--------
232633
 ldap/servers/plugins/replication/repl_extop.c |  19 +-
232633
 4 files changed, 299 insertions(+), 118 deletions(-)
232633
232633
diff --git a/dirsrvtests/tests/suites/replication/cleanallruv_test.py b/dirsrvtests/tests/suites/replication/cleanallruv_test.py
232633
index 09805d6b2..4893b81fe 100644
232633
--- a/dirsrvtests/tests/suites/replication/cleanallruv_test.py
232633
+++ b/dirsrvtests/tests/suites/replication/cleanallruv_test.py
232633
@@ -1,5 +1,5 @@
232633
 # --- BEGIN COPYRIGHT BLOCK ---
232633
-# Copyright (C) 2016 Red Hat, Inc.
232633
+# Copyright (C) 2019 Red Hat, Inc.
232633
 # All rights reserved.
232633
 #
232633
 # License: GPL (version 3 or any later version).
232633
@@ -7,7 +7,6 @@
232633
 # --- END COPYRIGHT BLOCK ---
232633
 #
232633
 import threading
232633
-
232633
 import pytest
232633
 import random
232633
 from lib389 import DirSrv
232633
@@ -721,6 +720,147 @@ def test_multiple_tasks_with_force(topology_m4, m4rid):
232633
         log.fatal('test_abort: CleanAllRUV task was not aborted')
232633
         assert False
232633
 
232633
+
232633
+@pytest.mark.bz1466441
232633
+@pytest.mark.ds50370
232633
+def test_clean_shutdown_crash(topology_m2):
232633
+    """Check that server didn't crash after shutdown when running CleanAllRUV task
232633
+
232633
+    :id: c34d0b40-3c3e-4f53-8656-5e4c2a310aaf
232633
+    :setup: Replication setup with two masters
232633
+    :steps:
232633
+        1. Enable TLS on both masters
232633
+        2. Reconfigure both agreements to use TLS Client auth
232633
+        3. Stop master2
232633
+        4. Run the CleanAllRUV task
232633
+        5. Restart master1
232633
+        6. Check if master1 didn't crash
232633
+        7. Restart master1 again
232633
+        8. Check if master1 didn't crash
232633
+
232633
+    :expectedresults:
232633
+        1. Success
232633
+        2. Success
232633
+        3. Success
232633
+        4. Success
232633
+        5. Success
232633
+        6. Success
232633
+        7. Success
232633
+        8. Success
232633
+    """
232633
+
232633
+    m1 = topology_m2.ms["master1"]
232633
+    m2 = topology_m2.ms["master2"]
232633
+
232633
+    repl = ReplicationManager(DEFAULT_SUFFIX)
232633
+
232633
+    cm_m1 = CertmapLegacy(m1)
232633
+    cm_m2 = CertmapLegacy(m2)
232633
+
232633
+    certmaps = cm_m1.list()
232633
+    certmaps['default']['DNComps'] = None
232633
+    certmaps['default']['CmapLdapAttr'] = 'nsCertSubjectDN'
232633
+
232633
+    cm_m1.set(certmaps)
232633
+    cm_m2.set(certmaps)
232633
+
232633
+    log.info('Enabling TLS')
232633
+    [i.enable_tls() for i in topology_m2]
232633
+
232633
+    log.info('Creating replication dns')
232633
+    services = ServiceAccounts(m1, DEFAULT_SUFFIX)
232633
+    repl_m1 = services.get('%s:%s' % (m1.host, m1.sslport))
232633
+    repl_m1.set('nsCertSubjectDN', m1.get_server_tls_subject())
232633
+
232633
+    repl_m2 = services.get('%s:%s' % (m2.host, m2.sslport))
232633
+    repl_m2.set('nsCertSubjectDN', m2.get_server_tls_subject())
232633
+
232633
+    log.info('Changing auth type')
232633
+    replica_m1 = Replicas(m1).get(DEFAULT_SUFFIX)
232633
+    agmt_m1 = replica_m1.get_agreements().list()[0]
232633
+    agmt_m1.replace_many(
232633
+        ('nsDS5ReplicaBindMethod', 'SSLCLIENTAUTH'),
232633
+        ('nsDS5ReplicaTransportInfo', 'SSL'),
232633
+        ('nsDS5ReplicaPort', '%s' % m2.sslport),
232633
+    )
232633
+
232633
+    agmt_m1.remove_all('nsDS5ReplicaBindDN')
232633
+
232633
+    replica_m2 = Replicas(m2).get(DEFAULT_SUFFIX)
232633
+    agmt_m2 = replica_m2.get_agreements().list()[0]
232633
+
232633
+    agmt_m2.replace_many(
232633
+        ('nsDS5ReplicaBindMethod', 'SSLCLIENTAUTH'),
232633
+        ('nsDS5ReplicaTransportInfo', 'SSL'),
232633
+        ('nsDS5ReplicaPort', '%s' % m1.sslport),
232633
+    )
232633
+    agmt_m2.remove_all('nsDS5ReplicaBindDN')
232633
+
232633
+    log.info('Stopping master2')
232633
+    m2.stop()
232633
+
232633
+    log.info('Run the cleanAllRUV task')
232633
+    cruv_task = CleanAllRUVTask(m1)
232633
+    cruv_task.create(properties={
232633
+        'replica-id': repl.get_rid(m1),
232633
+        'replica-base-dn': DEFAULT_SUFFIX,
232633
+        'replica-force-cleaning': 'no',
232633
+        'replica-certify-all': 'yes'
232633
+    })
232633
+
232633
+    m1.restart()
232633
+
232633
+    log.info('Check if master1 crashed')
232633
+    assert not m1.detectDisorderlyShutdown()
232633
+
232633
+    log.info('Repeat')
232633
+    m1.restart()
232633
+    assert not m1.detectDisorderlyShutdown()
232633
+
232633
+
232633
+def test_max_tasks(topology_m4):
232633
+    """Test we can not create more than 64 cleaning tasks
232633
+
232633
+    :id: c34d0b40-3c3e-4f53-8656-5e4c2a310a1f
232633
+    :setup: Replication setup with four masters
232633
+    :steps:
232633
+        1. Stop masters 3 & 4
232633
+        2. Create over 64 tasks between m1 and m2
232633
+        3. Check logs to see if (>65) tasks were rejected
232633
+
232633
+    :expectedresults:
232633
+        1. Success
232633
+        2. Success
232633
+        3. Success
232633
+    """
232633
+
232633
+    # Stop masters 3 & 4
232633
+    m1 = topology_m4.ms["master1"]
232633
+    m2 = topology_m4.ms["master2"]
232633
+    m3 = topology_m4.ms["master3"]
232633
+    m4 = topology_m4.ms["master4"]
232633
+    m3.stop()
232633
+    m4.stop()
232633
+
232633
+    # Add over 64 tasks between master1 & 2 to try to exceed the 64 task limit
232633
+    for i in range(1, 64):
232633
+        cruv_task = CleanAllRUVTask(m1)
232633
+        cruv_task.create(properties={
232633
+            'replica-id': str(i),
232633
+            'replica-base-dn': DEFAULT_SUFFIX,
232633
+            'replica-force-cleaning': 'no',  # This forces these tasks to stick around
232633
+        })
232633
+        cruv_task = CleanAllRUVTask(m2)
232633
+        cruv_task.create(properties={
232633
+            'replica-id': "10" + str(i),
232633
+            'replica-base-dn': DEFAULT_SUFFIX,
232633
+            'replica-force-cleaning': 'yes',  # This allows the tasks to propagate
232633
+        })
232633
+
232633
+    # Check the errors log for our error message in master 1
232633
+    assert m1.searchErrorsLog('Exceeded maximum number of active CLEANALLRUV tasks')
232633
+
232633
+
232633
 if __name__ == '__main__':
232633
     # Run isolated
232633
     # -s for DEBUG mode
232633
diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
232633
index 1801a333e..9d25f2305 100644
232633
--- a/ldap/servers/plugins/replication/repl5.h
232633
+++ b/ldap/servers/plugins/replication/repl5.h
232633
@@ -80,6 +80,8 @@
232633
 #define CLEANRUV_FINISHED  "finished"
232633
 #define CLEANRUV_CLEANING  "cleaning"
232633
 #define CLEANRUV_NO_MAXCSN "no maxcsn"
232633
+#define CLEANALLRUV_ID "CleanAllRUV Task"
232633
+#define ABORT_CLEANALLRUV_ID "Abort CleanAllRUV Task"
232633
 
232633
 /* DS 5.0 replication protocol error codes */
232633
 #define NSDS50_REPL_REPLICA_READY             0x00  /* Replica ready, go ahead */
232633
@@ -784,6 +786,7 @@ void multimaster_mtnode_construct_replicas(void);
232633
 void multimaster_be_state_change(void *handle, char *be_name, int old_be_state, int new_be_state);
232633
 
232633
 #define CLEANRIDSIZ 64 /* maximum number for concurrent CLEANALLRUV tasks */
232633
+#define CLEANRID_BUFSIZ 128
232633
 
232633
 typedef struct _cleanruv_data
232633
 {
232633
@@ -815,6 +818,8 @@ int get_replica_type(Replica *r);
232633
 int replica_execute_cleanruv_task_ext(Object *r, ReplicaId rid);
232633
 void add_cleaned_rid(cleanruv_data *data);
232633
 int is_cleaned_rid(ReplicaId rid);
232633
+int32_t check_and_set_cleanruv_task_count(ReplicaId rid);
232633
+int32_t check_and_set_abort_cleanruv_task_count(void);
232633
 int replica_cleanall_ruv_abort(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *eAfter, int *returncode, char *returntext, void *arg);
232633
 void replica_cleanallruv_thread_ext(void *arg);
232633
 void stop_ruv_cleaning(void);
232633
@@ -833,8 +838,6 @@ void set_cleaned_rid(ReplicaId rid);
232633
 void cleanruv_log(Slapi_Task *task, int rid, char *task_type, int sev_level, char *fmt, ...);
232633
 char *replica_cleanallruv_get_local_maxcsn(ReplicaId rid, char *base_dn);
232633
 
232633
-
232633
-
232633
 /* replutil.c */
232633
 LDAPControl *create_managedsait_control(void);
232633
 LDAPControl *create_backend_control(Slapi_DN *sdn);
232633
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
232633
index 749e90936..c66a1c81d 100644
232633
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
232633
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
232633
@@ -30,17 +30,18 @@
232633
 #define CLEANALLRUV "CLEANALLRUV"
232633
 #define CLEANALLRUVLEN 11
232633
 #define REPLICA_RDN "cn=replica"
232633
-#define CLEANALLRUV_ID "CleanAllRUV Task"
232633
-#define ABORT_CLEANALLRUV_ID "Abort CleanAllRUV Task"
232633
 
232633
 int slapi_log_urp = SLAPI_LOG_REPL;
232633
-static ReplicaId cleaned_rids[CLEANRIDSIZ + 1] = {0};
232633
-static ReplicaId pre_cleaned_rids[CLEANRIDSIZ + 1] = {0};
232633
-static ReplicaId aborted_rids[CLEANRIDSIZ + 1] = {0};
232633
-static Slapi_RWLock *rid_lock = NULL;
232633
-static Slapi_RWLock *abort_rid_lock = NULL;
232633
+static ReplicaId cleaned_rids[CLEANRID_BUFSIZ] = {0};
232633
+static ReplicaId pre_cleaned_rids[CLEANRID_BUFSIZ] = {0};
232633
+static ReplicaId aborted_rids[CLEANRID_BUFSIZ] = {0};
232633
+static PRLock *rid_lock = NULL;
232633
+static PRLock *abort_rid_lock = NULL;
232633
 static PRLock *notify_lock = NULL;
232633
 static PRCondVar *notify_cvar = NULL;
232633
+static PRLock *task_count_lock = NULL;
232633
+static int32_t clean_task_count = 0;
232633
+static int32_t abort_task_count = 0;
232633
 
232633
 /* Forward Declartions */
232633
 static int replica_config_add(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);
232633
@@ -67,8 +68,6 @@ static int replica_cleanallruv_send_abort_extop(Repl_Agmt *ra, Slapi_Task *task,
232633
 static int replica_cleanallruv_check_maxcsn(Repl_Agmt *agmt, char *basedn, char *rid_text, char *maxcsn, Slapi_Task *task);
232633
 static int replica_cleanallruv_replica_alive(Repl_Agmt *agmt);
232633
 static int replica_cleanallruv_check_ruv(char *repl_root, Repl_Agmt *ra, char *rid_text, Slapi_Task *task, char *force);
232633
-static int get_cleanruv_task_count(void);
232633
-static int get_abort_cleanruv_task_count(void);
232633
 static int replica_cleanup_task(Object *r, const char *task_name, char *returntext, int apply_mods);
232633
 static int replica_task_done(Replica *replica);
232633
 static void delete_cleaned_rid_config(cleanruv_data *data);
232633
@@ -114,20 +113,27 @@ replica_config_init()
232633
                       PR_GetError());
232633
         return -1;
232633
     }
232633
-    rid_lock = slapi_new_rwlock();
232633
+    rid_lock = PR_NewLock();
232633
     if (rid_lock == NULL) {
232633
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_init - "
232633
                                                        "Failed to create rid_lock; NSPR error - %d\n",
232633
                       PR_GetError());
232633
         return -1;
232633
     }
232633
-    abort_rid_lock = slapi_new_rwlock();
232633
+    abort_rid_lock = PR_NewLock();
232633
     if (abort_rid_lock == NULL) {
232633
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_init - "
232633
                                                        "Failed to create abort_rid_lock; NSPR error - %d\n",
232633
                       PR_GetError());
232633
         return -1;
232633
     }
232633
+    task_count_lock = PR_NewLock();
232633
+    if (task_count_lock == NULL) {
232633
+        slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_init - "
232633
+                                                       "Failed to create task_count_lock; NSPR error - %d\n",
232633
+                      PR_GetError());
232633
+        return -1;
232633
+    }
232633
     if ((notify_lock = PR_NewLock()) == NULL) {
232633
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_init - "
232633
                                                        "Failed to create notify lock; NSPR error - %d\n",
232633
@@ -1533,12 +1539,6 @@ replica_execute_cleanall_ruv_task(Object *r, ReplicaId rid, Slapi_Task *task, co
232633
 
232633
     cleanruv_log(pre_task, rid, CLEANALLRUV_ID, SLAPI_LOG_INFO, "Initiating CleanAllRUV Task...");
232633
 
232633
-    if (get_cleanruv_task_count() >= CLEANRIDSIZ) {
232633
-        /* we are already running the maximum number of tasks */
232633
-        cleanruv_log(pre_task, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR,
232633
-                     "Exceeded maximum number of active CLEANALLRUV tasks(%d)", CLEANRIDSIZ);
232633
-        return LDAP_UNWILLING_TO_PERFORM;
232633
-    }
232633
     /*
232633
      *  Grab the replica
232633
      */
232633
@@ -1590,6 +1590,13 @@ replica_execute_cleanall_ruv_task(Object *r, ReplicaId rid, Slapi_Task *task, co
232633
         goto fail;
232633
     }
232633
 
232633
+    if (check_and_set_cleanruv_task_count(rid) != LDAP_SUCCESS) {
232633
+        cleanruv_log(NULL, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR,
232633
+                     "Exceeded maximum number of active CLEANALLRUV tasks(%d)", CLEANRIDSIZ);
232633
+        rc = LDAP_UNWILLING_TO_PERFORM;
232633
+        goto fail;
232633
+    }
232633
+
232633
     /*
232633
      *  Launch the cleanallruv thread.  Once all the replicas are cleaned it will release the rid
232633
      */
232633
@@ -1597,6 +1604,9 @@ replica_execute_cleanall_ruv_task(Object *r, ReplicaId rid, Slapi_Task *task, co
232633
     if (data == NULL) {
232633
         cleanruv_log(pre_task, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR, "Failed to allocate cleanruv_data.  Aborting task.");
232633
         rc = -1;
232633
+        PR_Lock(task_count_lock);
232633
+        clean_task_count--;
232633
+        PR_Unlock(task_count_lock);
232633
         goto fail;
232633
     }
232633
     data->repl_obj = r;
232633
@@ -1679,13 +1689,13 @@ replica_cleanallruv_thread(void *arg)
232633
     int aborted = 0;
232633
     int rc = 0;
232633
 
232633
-    if (!data || slapi_is_shutting_down()) {
232633
-        return; /* no data */
232633
-    }
232633
-
232633
     /* Increase active thread count to prevent a race condition at server shutdown */
232633
     g_incr_active_threadcnt();
232633
 
232633
+    if (!data || slapi_is_shutting_down()) {
232633
+        goto done;
232633
+    }
232633
+
232633
     if (data->task) {
232633
         slapi_task_inc_refcount(data->task);
232633
         slapi_log_err(SLAPI_LOG_PLUGIN, repl_plugin_name,
232633
@@ -1732,16 +1742,13 @@ replica_cleanallruv_thread(void *arg)
232633
         slapi_task_begin(data->task, 1);
232633
     }
232633
     /*
232633
-     *  Presetting the rid prevents duplicate thread creation, but allows the db and changelog to still
232633
-     *  process updates from the rid.
232633
-     *  set_cleaned_rid() blocks updates, so we don't want to do that... yet unless we are in force mode.
232633
-     *  If we are forcing a clean independent of state of other servers for this RID we can set_cleaned_rid()
232633
+     *  We have already preset this rid, but if we are forcing a clean independent of state
232633
+     *  of other servers for this RID we can set_cleaned_rid()
232633
      */
232633
     if (data->force) {
232633
         set_cleaned_rid(data->rid);
232633
-    } else {
232633
-        preset_cleaned_rid(data->rid);
232633
     }
232633
+
232633
     rid_text = slapi_ch_smprintf("%d", data->rid);
232633
     csn_as_string(data->maxcsn, PR_FALSE, csnstr);
232633
     /*
232633
@@ -1911,6 +1918,9 @@ done:
232633
     /*
232633
      *  If the replicas are cleaned, release the rid
232633
      */
232633
+    if (slapi_is_shutting_down()) {
232633
+        stop_ruv_cleaning();
232633
+    }
232633
     if (!aborted && !slapi_is_shutting_down()) {
232633
         /*
232633
          * Success - the rid has been cleaned!
232633
@@ -1929,10 +1939,9 @@ done:
232633
         } else {
232633
             cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_INFO, "Propagated task does not delete Keep alive entry (%d).", data->rid);
232633
         }
232633
-
232633
         clean_agmts(data);
232633
         remove_cleaned_rid(data->rid);
232633
-        cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_INFO, "Successfully cleaned rid(%d).", data->rid);
232633
+        cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_INFO, "Successfully cleaned rid(%d)", data->rid);
232633
     } else {
232633
         /*
232633
          *  Shutdown or abort
232633
@@ -1965,6 +1974,10 @@ done:
232633
     slapi_ch_free_string(&data->force);
232633
     slapi_ch_free_string(&rid_text);
232633
     slapi_ch_free((void **)&data);
232633
+    /* decrement task count */
232633
+    PR_Lock(task_count_lock);
232633
+    clean_task_count--;
232633
+    PR_Unlock(task_count_lock);
232633
     g_decr_active_threadcnt();
232633
 }
232633
 
232633
@@ -2462,16 +2475,14 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, cleanruv_data *clean_data)
232633
 int
232633
 is_cleaned_rid(ReplicaId rid)
232633
 {
232633
-    int i;
232633
-
232633
-    slapi_rwlock_rdlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ && cleaned_rids[i] != 0; i++) {
232633
+    PR_Lock(rid_lock);
232633
+    for (size_t i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
         if (rid == cleaned_rids[i]) {
232633
-            slapi_rwlock_unlock(rid_lock);
232633
+            PR_Unlock(rid_lock);
232633
             return 1;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(rid_lock);
232633
 
232633
     return 0;
232633
 }
232633
@@ -2479,16 +2490,14 @@ is_cleaned_rid(ReplicaId rid)
232633
 int
232633
 is_pre_cleaned_rid(ReplicaId rid)
232633
 {
232633
-    int i;
232633
-
232633
-    slapi_rwlock_rdlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ && pre_cleaned_rids[i] != 0; i++) {
232633
+    PR_Lock(rid_lock);
232633
+    for (size_t i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
         if (rid == pre_cleaned_rids[i]) {
232633
-            slapi_rwlock_unlock(rid_lock);
232633
+            PR_Unlock(rid_lock);
232633
             return 1;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(rid_lock);
232633
 
232633
     return 0;
232633
 }
232633
@@ -2501,14 +2510,14 @@ is_task_aborted(ReplicaId rid)
232633
     if (rid == 0) {
232633
         return 0;
232633
     }
232633
-    slapi_rwlock_rdlock(abort_rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ && aborted_rids[i] != 0; i++) {
232633
+    PR_Lock(abort_rid_lock);
232633
+    for (i = 0; i < CLEANRID_BUFSIZ && aborted_rids[i] != 0; i++) {
232633
         if (rid == aborted_rids[i]) {
232633
-            slapi_rwlock_unlock(abort_rid_lock);
232633
+            PR_Unlock(abort_rid_lock);
232633
             return 1;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(abort_rid_lock);
232633
+    PR_Unlock(abort_rid_lock);
232633
     return 0;
232633
 }
232633
 
232633
@@ -2517,15 +2526,14 @@ preset_cleaned_rid(ReplicaId rid)
232633
 {
232633
     int i;
232633
 
232633
-    slapi_rwlock_wrlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ; i++) {
232633
+    PR_Lock(rid_lock);
232633
+    for (i = 0; i < CLEANRID_BUFSIZ && pre_cleaned_rids[i] != rid; i++) {
232633
         if (pre_cleaned_rids[i] == 0) {
232633
             pre_cleaned_rids[i] = rid;
232633
-            pre_cleaned_rids[i + 1] = 0;
232633
             break;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(rid_lock);
232633
 }
232633
 
232633
 /*
232633
@@ -2538,14 +2546,13 @@ set_cleaned_rid(ReplicaId rid)
232633
 {
232633
     int i;
232633
 
232633
-    slapi_rwlock_wrlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ; i++) {
232633
+    PR_Lock(rid_lock);
232633
+    for (i = 0; i < CLEANRID_BUFSIZ && cleaned_rids[i] != rid; i++) {
232633
         if (cleaned_rids[i] == 0) {
232633
             cleaned_rids[i] = rid;
232633
-            cleaned_rids[i + 1] = 0;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(rid_lock);
232633
 }
232633
 
232633
 /*
232633
@@ -2621,15 +2628,14 @@ add_aborted_rid(ReplicaId rid, Replica *r, char *repl_root, char *certify_all, P
232633
     int rc;
232633
     int i;
232633
 
232633
-    slapi_rwlock_wrlock(abort_rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ; i++) {
232633
+    PR_Lock(abort_rid_lock);
232633
+    for (i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
         if (aborted_rids[i] == 0) {
232633
             aborted_rids[i] = rid;
232633
-            aborted_rids[i + 1] = 0;
232633
             break;
232633
         }
232633
     }
232633
-    slapi_rwlock_unlock(abort_rid_lock);
232633
+    PR_Unlock(abort_rid_lock);
232633
     /*
232633
      *  Write the rid to the config entry
232633
      */
232633
@@ -2672,21 +2678,24 @@ delete_aborted_rid(Replica *r, ReplicaId rid, char *repl_root, char *certify_all
232633
     char *data;
232633
     char *dn;
232633
     int rc;
232633
-    int i;
232633
 
232633
     if (r == NULL)
232633
         return;
232633
 
232633
     if (skip) {
232633
         /* skip the deleting of the config, and just remove the in memory rid */
232633
-        slapi_rwlock_wrlock(abort_rid_lock);
232633
-        for (i = 0; i < CLEANRIDSIZ && aborted_rids[i] != rid; i++)
232633
-            ; /* found rid, stop */
232633
-        for (; i < CLEANRIDSIZ; i++) {
232633
-            /* rewrite entire array */
232633
-            aborted_rids[i] = aborted_rids[i + 1];
232633
-        }
232633
-        slapi_rwlock_unlock(abort_rid_lock);
232633
+        ReplicaId new_abort_rids[CLEANRID_BUFSIZ] = {0};
232633
+        int32_t idx = 0;
232633
+
232633
+        PR_Lock(abort_rid_lock);
232633
+        for (size_t i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
+            if (aborted_rids[i] != rid) {
232633
+                new_abort_rids[idx] = aborted_rids[i];
232633
+                idx++;
232633
+            }
232633
+        }
232633
+        memcpy(aborted_rids, new_abort_rids, sizeof(new_abort_rids));
232633
+        PR_Unlock(abort_rid_lock);
232633
     } else {
232633
         /* only remove the config, leave the in-memory rid */
232633
         dn = replica_get_dn(r);
232633
@@ -2832,27 +2841,31 @@ bail:
232633
 void
232633
 remove_cleaned_rid(ReplicaId rid)
232633
 {
232633
-    int i;
232633
-    /*
232633
-     *  Remove this rid, and optimize the array
232633
-     */
232633
-    slapi_rwlock_wrlock(rid_lock);
232633
+    ReplicaId new_cleaned_rids[CLEANRID_BUFSIZ] = {0};
232633
+    ReplicaId new_pre_cleaned_rids[CLEANRID_BUFSIZ] = {0};
232633
+    size_t idx = 0;
232633
+
232633
+    PR_Lock(rid_lock);
232633
 
232633
-    for (i = 0; i < CLEANRIDSIZ && cleaned_rids[i] != rid; i++)
232633
-        ; /* found rid, stop */
232633
-    for (; i < CLEANRIDSIZ; i++) {
232633
-        /* rewrite entire array */
232633
-        cleaned_rids[i] = cleaned_rids[i + 1];
232633
+    for (size_t i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
+        if (cleaned_rids[i] != rid) {
232633
+            new_cleaned_rids[idx] = cleaned_rids[i];
232633
+            idx++;
232633
+        }
232633
     }
232633
+    memcpy(cleaned_rids, new_cleaned_rids, sizeof(new_cleaned_rids));
232633
+
232633
     /* now do the preset cleaned rids */
232633
-    for (i = 0; i < CLEANRIDSIZ && pre_cleaned_rids[i] != rid; i++)
232633
-        ; /* found rid, stop */
232633
-    for (; i < CLEANRIDSIZ; i++) {
232633
-        /* rewrite entire array */
232633
-        pre_cleaned_rids[i] = pre_cleaned_rids[i + 1];
232633
+    idx = 0;
232633
+    for (size_t i = 0; i < CLEANRID_BUFSIZ; i++) {
232633
+        if (pre_cleaned_rids[i] != rid) {
232633
+            new_pre_cleaned_rids[idx] = pre_cleaned_rids[i];
232633
+            idx++;
232633
+        }
232633
     }
232633
+    memcpy(pre_cleaned_rids, new_pre_cleaned_rids, sizeof(new_pre_cleaned_rids));
232633
 
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(rid_lock);
232633
 }
232633
 
232633
 /*
232633
@@ -2882,16 +2895,6 @@ replica_cleanall_ruv_abort(Slapi_PBlock *pb __attribute__((unused)),
232633
     char *ridstr = NULL;
232633
     int rc = SLAPI_DSE_CALLBACK_OK;
232633
 
232633
-    if (get_abort_cleanruv_task_count() >= CLEANRIDSIZ) {
232633
-        /* we are already running the maximum number of tasks */
232633
-        PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
232633
-                    "Exceeded maximum number of active ABORT CLEANALLRUV tasks(%d)",
232633
-                    CLEANRIDSIZ);
232633
-        cleanruv_log(task, -1, ABORT_CLEANALLRUV_ID, SLAPI_LOG_ERR, "%s", returntext);
232633
-        *returncode = LDAP_OPERATIONS_ERROR;
232633
-        return SLAPI_DSE_CALLBACK_ERROR;
232633
-    }
232633
-
232633
     /* allocate new task now */
232633
     task = slapi_new_task(slapi_entry_get_ndn(e));
232633
 
232633
@@ -2976,6 +2979,16 @@ replica_cleanall_ruv_abort(Slapi_PBlock *pb __attribute__((unused)),
232633
          */
232633
         certify_all = "no";
232633
     }
232633
+
232633
+    if (check_and_set_abort_cleanruv_task_count() != LDAP_SUCCESS) {
232633
+        /* we are already running the maximum number of tasks */
232633
+        PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,
232633
+                    "Exceeded maximum number of active ABORT CLEANALLRUV tasks(%d)",
232633
+                    CLEANRIDSIZ);
232633
+        cleanruv_log(task, -1, ABORT_CLEANALLRUV_ID, SLAPI_LOG_ERR, "%s", returntext);
232633
+        *returncode = LDAP_UNWILLING_TO_PERFORM;
232633
+        goto out;
232633
+    }
232633
     /*
232633
      *  Create payload
232633
      */
232633
@@ -3190,6 +3203,9 @@ done:
232633
     slapi_ch_free_string(&data->certify);
232633
     slapi_sdn_free(&data->sdn);
232633
     slapi_ch_free((void **)&data);
232633
+    PR_Lock(task_count_lock);
232633
+    abort_task_count--;
232633
+    PR_Unlock(task_count_lock);
232633
     g_decr_active_threadcnt();
232633
 }
232633
 
232633
@@ -3541,36 +3557,43 @@ replica_cleanallruv_check_ruv(char *repl_root, Repl_Agmt *agmt, char *rid_text,
232633
     return rc;
232633
 }
232633
 
232633
-static int
232633
-get_cleanruv_task_count(void)
232633
+/*
232633
+ * Before starting a cleanAllRUV task make sure there are not
232633
+ * too many task threads already running.  If everything is okay
232633
+ * also pre-set the RID now so rebounding extended ops do not
232633
+ * try to clean it over and over.
232633
+ */
232633
+int32_t
232633
+check_and_set_cleanruv_task_count(ReplicaId rid)
232633
 {
232633
-    int i, count = 0;
232633
+    int32_t rc = 0;
232633
 
232633
-    slapi_rwlock_wrlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ; i++) {
232633
-        if (pre_cleaned_rids[i] != 0) {
232633
-            count++;
232633
-        }
232633
+    PR_Lock(task_count_lock);
232633
+    if (clean_task_count >= CLEANRIDSIZ) {
232633
+        rc = -1;
232633
+    } else {
232633
+        clean_task_count++;
232633
+        preset_cleaned_rid(rid);
232633
     }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(task_count_lock);
232633
 
232633
-    return count;
232633
+    return rc;
232633
 }
232633
 
232633
-static int
232633
-get_abort_cleanruv_task_count(void)
232633
+int32_t
232633
+check_and_set_abort_cleanruv_task_count(void)
232633
 {
232633
-    int i, count = 0;
232633
+    int32_t rc = 0;
232633
 
232633
-    slapi_rwlock_wrlock(rid_lock);
232633
-    for (i = 0; i < CLEANRIDSIZ; i++) {
232633
-        if (aborted_rids[i] != 0) {
232633
-            count++;
232633
+    PR_Lock(task_count_lock);
232633
+    if (abort_task_count > CLEANRIDSIZ) {
232633
+            rc = -1;
232633
+        } else {
232633
+            abort_task_count++;
232633
         }
232633
-    }
232633
-    slapi_rwlock_unlock(rid_lock);
232633
+    PR_Unlock(task_count_lock);
232633
 
232633
-    return count;
232633
+    return rc;
232633
 }
232633
 
232633
 /*
232633
diff --git a/ldap/servers/plugins/replication/repl_extop.c b/ldap/servers/plugins/replication/repl_extop.c
232633
index b49cb8cd5..5bed84958 100644
232633
--- a/ldap/servers/plugins/replication/repl_extop.c
232633
+++ b/ldap/servers/plugins/replication/repl_extop.c
232633
@@ -1393,6 +1393,12 @@ multimaster_extop_abort_cleanruv(Slapi_PBlock *pb)
232633
         rc = LDAP_OPERATIONS_ERROR;
232633
         goto out;
232633
     }
232633
+    if (check_and_set_abort_cleanruv_task_count() != LDAP_SUCCESS) {
232633
+        cleanruv_log(NULL, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR,
232633
+                     "Exceeded maximum number of active abort CLEANALLRUV tasks(%d)", CLEANRIDSIZ);
232633
+        rc = LDAP_UNWILLING_TO_PERFORM;
232633
+        goto out;
232633
+    }
232633
     /*
232633
      *  Prepare the abort data
232633
      */
232633
@@ -1499,6 +1505,7 @@ multimaster_extop_cleanruv(Slapi_PBlock *pb)
232633
     if (force == NULL) {
232633
         force = "no";
232633
     }
232633
+
232633
     maxcsn = csn_new();
232633
     csn_init_by_string(maxcsn, csnstr);
232633
     /*
232633
@@ -1535,13 +1542,21 @@ multimaster_extop_cleanruv(Slapi_PBlock *pb)
232633
         goto free_and_return;
232633
     }
232633
 
232633
+    if (check_and_set_cleanruv_task_count((ReplicaId)rid) != LDAP_SUCCESS) {
232633
+        cleanruv_log(NULL, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR,
232633
+                     "Exceeded maximum number of active CLEANALLRUV tasks(%d)", CLEANRIDSIZ);
232633
+        rc = LDAP_UNWILLING_TO_PERFORM;
232633
+        goto free_and_return;
232633
+    }
232633
+
232633
     if (replica_get_type(r) != REPLICA_TYPE_READONLY) {
232633
         /*
232633
          *  Launch the cleanruv monitoring thread.  Once all the replicas are cleaned it will release the rid
232633
          *
232633
          *  This will also release mtnode_ext->replica
232633
          */
232633
-        slapi_log_err(SLAPI_LOG_INFO, repl_plugin_name, "multimaster_extop_cleanruv - CleanAllRUV Task - Launching cleanAllRUV thread...\n");
232633
+
232633
+        cleanruv_log(NULL, rid, CLEANALLRUV_ID, SLAPI_LOG_ERR, "Launching cleanAllRUV thread...\n");
232633
         data = (cleanruv_data *)slapi_ch_calloc(1, sizeof(cleanruv_data));
232633
         if (data == NULL) {
232633
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "multimaster_extop_cleanruv - CleanAllRUV Task - Failed to allocate "
232633
@@ -1635,7 +1650,7 @@ free_and_return:
232633
         ber_printf(resp_bere, "{s}", CLEANRUV_ACCEPTED);
232633
         ber_flatten(resp_bere, &resp_bval);
232633
         slapi_pblock_set(pb, SLAPI_EXT_OP_RET_VALUE, resp_bval);
232633
-        slapi_send_ldap_result(pb, LDAP_SUCCESS, NULL, NULL, 0, NULL);
232633
+        slapi_send_ldap_result(pb, rc, NULL, NULL, 0, NULL);
232633
         /* resp_bere */
232633
         if (NULL != resp_bere) {
232633
             ber_free(resp_bere, 1);
232633
-- 
232633
2.21.0
232633