From 99dbba52eb45628c7f290e9ed3aeabb2a2a67db4 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Tue, 13 Dec 2022 09:41:34 -0500
Subject: [PATCH] Issue 5413 - Allow mutliple MemberOf fixup tasks with
different bases/filters
Description:
A change was made to only allow a single fixup task at a time, but there are
cases where you would want to run mutliple tasks but on different branches/filters.
Now we maintain a linked list of bases/filters of the current running tasks to
monitor this.
relates: https://github.com/389ds/389-ds-base/issues/5413
Reviewed by: tbordaz(Thanks!)
---
.../suites/memberof_plugin/fixup_test.py | 5 +-
ldap/servers/plugins/memberof/memberof.c | 101 ++++++++++++++----
2 files changed, 85 insertions(+), 21 deletions(-)
diff --git a/dirsrvtests/tests/suites/memberof_plugin/fixup_test.py b/dirsrvtests/tests/suites/memberof_plugin/fixup_test.py
index 9566e144c..d5369439f 100644
--- a/dirsrvtests/tests/suites/memberof_plugin/fixup_test.py
+++ b/dirsrvtests/tests/suites/memberof_plugin/fixup_test.py
@@ -59,12 +59,15 @@ def test_fixup_task_limit(topo):
with pytest.raises(ldap.UNWILLING_TO_PERFORM):
memberof.fixup(DEFAULT_SUFFIX)
+ # Add second task but on different suffix which should be allowed
+ memberof.fixup("ou=people," + DEFAULT_SUFFIX)
+
# Wait for first task to complete
task.wait()
# Add new task which should be allowed now
memberof.fixup(DEFAULT_SUFFIX)
-
+
if __name__ == '__main__':
# Run isolated
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index 0b8cfe95c..a14617044 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -52,7 +52,6 @@ static Slapi_DN* _pluginDN = NULL;
MemberOfConfig *qsortConfig = 0;
static int usetxn = 0;
static int premodfn = 0;
-static PRBool fixup_running = PR_FALSE;
static PRLock *fixup_lock = NULL;
static int32_t fixup_progress_count = 0;
static int64_t fixup_progress_elapsed = 0;
@@ -65,6 +64,15 @@ typedef struct _memberofstringll
void *next;
} memberofstringll;
+typedef struct _fixup_ll
+{
+ Slapi_DN *sdn;
+ char *filter_str;
+ void *next;
+} mo_fixup_ll;
+
+static mo_fixup_ll *fixup_list = NULL;
+
typedef struct _memberof_get_groups_data
{
MemberOfConfig *config;
@@ -438,6 +446,15 @@ memberof_postop_close(Slapi_PBlock *pb __attribute__((unused)))
PR_DestroyLock(fixup_lock);
fixup_lock = NULL;
+ mo_fixup_ll *fixup_task = fixup_list;
+ while (fixup_task != NULL) {
+ mo_fixup_ll *tmp = fixup_task;
+ fixup_task = fixup_task->next;
+ slapi_sdn_free(&tmp->sdn);
+ slapi_ch_free_string(&tmp->filter_str);
+ slapi_ch_free((void**)&tmp);
+ }
+
slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
"<-- memberof_postop_close\n");
return 0;
@@ -2817,7 +2834,6 @@ memberof_fixup_task_thread(void *arg)
}
PR_Lock(fixup_lock);
- fixup_running = PR_TRUE;
fixup_progress_count = 0;
fixup_progress_elapsed = slapi_current_rel_time_t();
fixup_start_time = slapi_current_rel_time_t();
@@ -2849,11 +2865,10 @@ memberof_fixup_task_thread(void *arg)
/* Mark this as a task operation */
configCopy.fixup_task = 1;
configCopy.task = task;
-
+ Slapi_DN *sdn = slapi_sdn_new_dn_byref(td->dn);
if (usetxn) {
- Slapi_DN *sdn = slapi_sdn_new_dn_byref(td->dn);
Slapi_Backend *be = slapi_be_select_exact(sdn);
- slapi_sdn_free(&sdn);
+
if (be) {
fixup_pb = slapi_pblock_new();
slapi_pblock_set(fixup_pb, SLAPI_BACKEND, be);
@@ -2894,14 +2909,37 @@ done:
fixup_progress_count, slapi_current_rel_time_t() - fixup_start_time);
slapi_task_inc_progress(task);
+ /* Cleanup task linked list */
+ PR_Lock(fixup_lock);
+ mo_fixup_ll *prev = NULL;
+ for (mo_fixup_ll *curr = fixup_list; curr; curr = curr->next) {
+ mo_fixup_ll *next = curr->next;
+ if (slapi_sdn_compare(curr->sdn, sdn) == 0 &&
+ strcasecmp(curr->filter_str, td->filter_str) == 0)
+ {
+ /* free current code */
+ slapi_sdn_free(&curr->sdn);
+ slapi_ch_free_string(&curr->filter_str);
+ slapi_ch_free((void**)&curr);
+
+ /* update linked list */
+ if (prev == NULL) {
+ /* first node */
+ fixup_list = next;
+ } else {
+ prev->next = next;
+ }
+ break;
+ }
+ prev = curr;
+ }
+ PR_Unlock(fixup_lock);
+ slapi_sdn_free(&sdn);
+
/* this will queue the destruction of the task */
slapi_task_finish(task, rc);
slapi_task_dec_refcount(task);
- PR_Lock(fixup_lock);
- fixup_running = PR_FALSE;
- PR_Unlock(fixup_lock);
-
slapi_log_err(SLAPI_LOG_INFO, MEMBEROF_PLUGIN_SUBSYSTEM,
"memberof_fixup_task_thread - Memberof task finished (processed %d entries in %ld seconds)\n",
fixup_progress_count, slapi_current_rel_time_t() - fixup_start_time);
@@ -2919,23 +2957,13 @@ memberof_task_add(Slapi_PBlock *pb,
int rv = SLAPI_DSE_CALLBACK_OK;
task_data *mytaskdata = NULL;
Slapi_Task *task = NULL;
+ Slapi_DN *sdn = NULL;
char *bind_dn;
const char *filter;
const char *dn = 0;
*returncode = LDAP_SUCCESS;
- PR_Lock(fixup_lock);
- if (fixup_running) {
- PR_Unlock(fixup_lock);
- *returncode = LDAP_UNWILLING_TO_PERFORM;
- slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM,
- "memberof_task_add - there is already a fixup task running\n");
- rv = SLAPI_DSE_CALLBACK_ERROR;
- goto out;
- }
- PR_Unlock(fixup_lock);
-
/* get arg(s) */
if ((dn = slapi_entry_attr_get_ref(e, "basedn")) == NULL) {
*returncode = LDAP_OBJECT_CLASS_VIOLATION;
@@ -2949,6 +2977,39 @@ memberof_task_add(Slapi_PBlock *pb,
goto out;
}
+ PR_Lock(fixup_lock);
+ sdn = slapi_sdn_new_dn_byval(dn);
+ if (fixup_list == NULL) {
+ fixup_list = (mo_fixup_ll *)slapi_ch_calloc(1, sizeof(mo_fixup_ll));
+ fixup_list->sdn = sdn;
+ fixup_list->filter_str = slapi_ch_strdup(filter);
+ } else {
+ for (mo_fixup_ll *fixup_task = fixup_list; fixup_task; fixup_task = fixup_task->next) {
+ if (slapi_sdn_compare(sdn, fixup_task->sdn) == 0 &&
+ strcasecmp(filter, fixup_task->filter_str) == 0)
+ {
+ /* Found an identical running task, reject it */
+ PR_Unlock(fixup_lock);
+ slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM,
+ "memberof_task_add - there is already an identical fixup task running: base: %s filter: %s\n",
+ slapi_sdn_get_dn(sdn), filter);
+ slapi_sdn_free(&sdn);
+ *returncode = LDAP_UNWILLING_TO_PERFORM;
+ rv = SLAPI_DSE_CALLBACK_ERROR;
+ goto out;
+ }
+ }
+ /* Add the new task DN to the top of the list */
+ mo_fixup_ll *head = fixup_list;
+ mo_fixup_ll *new_task = (mo_fixup_ll *)slapi_ch_calloc(1, sizeof(mo_fixup_ll));
+ new_task->sdn = sdn;
+ new_task->filter_str = slapi_ch_strdup(filter);
+ new_task->next = head;
+ fixup_list = new_task;
+ }
+ PR_Unlock(fixup_lock);
+
+
/* setup our task data */
slapi_pblock_get(pb, SLAPI_REQUESTOR_DN, &bind_dn);
mytaskdata = (task_data *)slapi_ch_malloc(sizeof(task_data));
--
2.38.1