Blame SOURCES/0008-Issue-50646-Improve-task-handling-during-shutdowns.patch

4c04d8
From 81587cabb358bb24d0ae2623076e09676a4a0620 Mon Sep 17 00:00:00 2001
4c04d8
From: Mark Reynolds <mreynolds@redhat.com>
4c04d8
Date: Tue, 15 Oct 2019 11:02:24 -0400
4c04d8
Subject: [PATCH] Issue 50646 - Improve task handling during shutdowns
4c04d8
4c04d8
Bug Description:  There is a race condition when stopping the server and
4c04d8
                  a running import task that can cause a heap-use-after-free.
4c04d8
4c04d8
Fix Description:  For an import task, encapsulate the import thread with
4c04d8
                  a global thread increment/decrement (just like the export
4c04d8
                  task).  Also improved how tasks are notified to abort by
4c04d8
                  notifiying them before we wait for active threads to finish.
4c04d8
                  Then the tasks get destroyed after all threads are complete.
4c04d8
4c04d8
relates: https://pagure.io/389-ds-base/issue/50646
4c04d8
4c04d8
Reviewed by: lkrispen & tbordaz (Thanks!!)
4c04d8
---
4c04d8
 ldap/servers/slapd/back-ldbm/import.c |  3 ++
4c04d8
 ldap/servers/slapd/daemon.c           |  5 ++++
4c04d8
 ldap/servers/slapd/main.c             |  2 +-
4c04d8
 ldap/servers/slapd/slapi-private.h    |  1 +
4c04d8
 ldap/servers/slapd/task.c             | 40 +++++++++++++++++----------
4c04d8
 5 files changed, 35 insertions(+), 16 deletions(-)
4c04d8
4c04d8
diff --git a/ldap/servers/slapd/back-ldbm/import.c b/ldap/servers/slapd/back-ldbm/import.c
4c04d8
index 42e2696d3..1c21f6e55 100644
4c04d8
--- a/ldap/servers/slapd/back-ldbm/import.c
4c04d8
+++ b/ldap/servers/slapd/back-ldbm/import.c
4c04d8
@@ -1626,7 +1626,10 @@ error:
4c04d8
 void
4c04d8
 import_main(void *arg)
4c04d8
 {
4c04d8
+    /* For online import tasks increment/decrement the global thread count */
4c04d8
+    g_incr_active_threadcnt();
4c04d8
     import_main_offline(arg);
4c04d8
+    g_decr_active_threadcnt();
4c04d8
 }
4c04d8
 
4c04d8
 int
4c04d8
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
4c04d8
index afe0fb737..5d8767df6 100644
4c04d8
--- a/ldap/servers/slapd/daemon.c
4c04d8
+++ b/ldap/servers/slapd/daemon.c
4c04d8
@@ -1218,6 +1218,11 @@ slapd_daemon(daemon_ports_t *ports, ns_thrpool_t *tp)
4c04d8
         ns_thrpool_wait(tp);
4c04d8
     }
4c04d8
 
4c04d8
+    if (!in_referral_mode) {
4c04d8
+        /* signal tasks to start shutting down */
4c04d8
+        task_cancel_all();
4c04d8
+    }
4c04d8
+
4c04d8
     threads = g_get_active_threadcnt();
4c04d8
     if (threads > 0) {
4c04d8
         slapi_log_err(SLAPI_LOG_INFO, "slapd_daemon",
4c04d8
diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c
4c04d8
index 5e24b3b5f..5ca52ce74 100644
4c04d8
--- a/ldap/servers/slapd/main.c
4c04d8
+++ b/ldap/servers/slapd/main.c
4c04d8
@@ -1989,7 +1989,7 @@ lookup_plugin_by_instance_name(const char *name)
4c04d8
 {
4c04d8
     Slapi_Entry **entries = NULL;
4c04d8
     Slapi_PBlock *pb = slapi_pblock_new();
4c04d8
-    struct slapdplugin *plugin;
4c04d8
+    struct slapdplugin *plugin = NULL;
4c04d8
     char *query, *dn, *cn;
4c04d8
     int ret = 0;
4c04d8
 
4c04d8
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
4c04d8
index b347b6162..d676486a8 100644
4c04d8
--- a/ldap/servers/slapd/slapi-private.h
4c04d8
+++ b/ldap/servers/slapd/slapi-private.h
4c04d8
@@ -794,6 +794,7 @@ int slapi_lookup_instance_name_by_suffix(char *suffix,
4c04d8
 
4c04d8
 /* begin and end the task subsystem */
4c04d8
 void task_init(void);
4c04d8
+void task_cancel_all(void);
4c04d8
 void task_shutdown(void);
4c04d8
 void task_cleanup(void);
4c04d8
 
4c04d8
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
4c04d8
index 80a238392..7e7094750 100644
4c04d8
--- a/ldap/servers/slapd/task.c
4c04d8
+++ b/ldap/servers/slapd/task.c
4c04d8
@@ -26,7 +26,7 @@
4c04d8
  */
4c04d8
 static Slapi_Task *global_task_list = NULL;
4c04d8
 static PRLock *global_task_lock = NULL;
4c04d8
-static int shutting_down = 0;
4c04d8
+static uint64_t shutting_down = 0;
4c04d8
 
4c04d8
 /***********************************
4c04d8
  * Private Defines
4c04d8
@@ -588,7 +588,7 @@ new_task(const char *rawdn, void *plugin)
4c04d8
     Slapi_Task *task = NULL;
4c04d8
     char *dn = NULL;
4c04d8
 
4c04d8
-    if (rawdn == NULL) {
4c04d8
+    if (rawdn == NULL || shutting_down) {
4c04d8
         return NULL;
4c04d8
     }
4c04d8
 
4c04d8
@@ -600,9 +600,20 @@ new_task(const char *rawdn, void *plugin)
4c04d8
     }
4c04d8
     task = (Slapi_Task *)slapi_ch_calloc(1, sizeof(Slapi_Task));
4c04d8
     PR_Lock(global_task_lock);
4c04d8
+    if (shutting_down) {
4c04d8
+        /* Abort!  Free everything and return NULL */
4c04d8
+        PR_Unlock(task->task_log_lock);
4c04d8
+        PR_Unlock(global_task_lock);
4c04d8
+        PR_DestroyLock(task->task_log_lock);
4c04d8
+        slapi_ch_free((void **)&task);
4c04d8
+        slapi_ch_free_string(&dn);
4c04d8
+        slapi_log_err(SLAPI_LOG_ERR, "new_task", "Server is shutting down, aborting task: %s\n", rawdn);
4c04d8
+        return NULL;
4c04d8
+    }
4c04d8
     task->next = global_task_list;
4c04d8
     global_task_list = task;
4c04d8
     PR_Unlock(global_task_lock);
4c04d8
+
4c04d8
     task->task_dn = dn;
4c04d8
     task->task_state = SLAPI_TASK_SETUP;
4c04d8
     task->task_flags = SLAPI_TASK_RUNNING_AS_TASK;
4c04d8
@@ -2990,32 +3001,31 @@ task_init(void)
4c04d8
 
4c04d8
 /* called when the server is shutting down -- abort all existing tasks */
4c04d8
 void
4c04d8
-task_shutdown(void)
4c04d8
-{
4c04d8
+task_cancel_all(void) {
4c04d8
     Slapi_Task *task;
4c04d8
-    int found_any = 0;
4c04d8
 
4c04d8
-    /* first, cancel all tasks */
4c04d8
     PR_Lock(global_task_lock);
4c04d8
     shutting_down = 1;
4c04d8
     for (task = global_task_list; task; task = task->next) {
4c04d8
-        if ((task->task_state != SLAPI_TASK_CANCELLED) &&
4c04d8
-            (task->task_state != SLAPI_TASK_FINISHED)) {
4c04d8
+        if (task->task_state != SLAPI_TASK_CANCELLED &&
4c04d8
+            task->task_state != SLAPI_TASK_FINISHED)
4c04d8
+        {
4c04d8
             task->task_state = SLAPI_TASK_CANCELLED;
4c04d8
             if (task->cancel) {
4c04d8
-                slapi_log_err(SLAPI_LOG_INFO, "task_shutdown", "Cancelling task '%s'\n",
4c04d8
+                slapi_log_err(SLAPI_LOG_INFO, "task_cancel_all", "Canceling task '%s'\n",
4c04d8
                               task->task_dn);
4c04d8
                 (*task->cancel)(task);
4c04d8
-                found_any = 1;
4c04d8
             }
4c04d8
         }
4c04d8
     }
4c04d8
+    PR_Unlock(global_task_lock);
4c04d8
+}
4c04d8
 
4c04d8
-    if (found_any) {
4c04d8
-        /* give any tasks 1 second to say their last rites */
4c04d8
-        DS_Sleep(PR_SecondsToInterval(1));
4c04d8
-    }
4c04d8
-
4c04d8
+void
4c04d8
+task_shutdown(void)
4c04d8
+{
4c04d8
+    /* Now we can destroy the tasks... */
4c04d8
+    PR_Lock(global_task_lock);
4c04d8
     while (global_task_list) {
4c04d8
         destroy_task(0, global_task_list);
4c04d8
     }
4c04d8
-- 
4c04d8
2.21.0
4c04d8