This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.0
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.0 by this push:
new 4898470 Issue 50646 - Improve task handling during shutdowns
4898470 is described below
commit 48984709eed2d0fa6b4f7cb6898b94fe3ed4deac
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Tue Oct 15 11:02:24 2019 -0400
Issue 50646 - Improve task handling during shutdowns
Bug Description: There is a race condition when stopping the server and
a running import task that can cause a heap-use-after-free.
Fix Description: For an import task, encapsulate the import thread with
a global thread increment/decrement (just like the export
task). Also improved how tasks are notified to abort by
notifiying them before we wait for active threads to finish.
Then the tasks get destroyed after all threads are complete.
relates:
https://pagure.io/389-ds-base/issue/50646
Reviewed by: lkrispen & tbordaz (Thanks!!)
---
ldap/servers/slapd/back-ldbm/import.c | 3 +++
ldap/servers/slapd/daemon.c | 5 +++++
ldap/servers/slapd/main.c | 2 +-
ldap/servers/slapd/slapi-private.h | 1 +
ldap/servers/slapd/task.c | 40 ++++++++++++++++++++++-------------
5 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/ldap/servers/slapd/back-ldbm/import.c
b/ldap/servers/slapd/back-ldbm/import.c
index 42e2696..1c21f6e 100644
--- a/ldap/servers/slapd/back-ldbm/import.c
+++ b/ldap/servers/slapd/back-ldbm/import.c
@@ -1626,7 +1626,10 @@ error:
void
import_main(void *arg)
{
+ /* For online import tasks increment/decrement the global thread count */
+ g_incr_active_threadcnt();
import_main_offline(arg);
+ g_decr_active_threadcnt();
}
int
diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c
index b1d41c8..9f9c3a7 100644
--- a/ldap/servers/slapd/daemon.c
+++ b/ldap/servers/slapd/daemon.c
@@ -1218,6 +1218,11 @@ slapd_daemon(daemon_ports_t *ports, ns_thrpool_t *tp)
ns_thrpool_wait(tp);
}
+ if (!in_referral_mode) {
+ /* signal tasks to start shutting down */
+ task_cancel_all();
+ }
+
threads = g_get_active_threadcnt();
if (threads > 0) {
slapi_log_err(SLAPI_LOG_INFO, "slapd_daemon",
diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c
index 8224cd0..bd8db5f 100644
--- a/ldap/servers/slapd/main.c
+++ b/ldap/servers/slapd/main.c
@@ -2002,7 +2002,7 @@ lookup_plugin_by_instance_name(const char *name)
{
Slapi_Entry **entries = NULL;
Slapi_PBlock *pb = slapi_pblock_new();
- struct slapdplugin *plugin;
+ struct slapdplugin *plugin = NULL;
char *query, *dn, *cn;
int ret = 0;
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
index 4161002..b4e7028 100644
--- a/ldap/servers/slapd/slapi-private.h
+++ b/ldap/servers/slapd/slapi-private.h
@@ -794,6 +794,7 @@ int slapi_lookup_instance_name_by_suffix(char *suffix,
/* begin and end the task subsystem */
void task_init(void);
+void task_cancel_all(void);
void task_shutdown(void);
void task_cleanup(void);
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
index 025ffcb..31a6f2b 100644
--- a/ldap/servers/slapd/task.c
+++ b/ldap/servers/slapd/task.c
@@ -26,7 +26,7 @@
*/
static Slapi_Task *global_task_list = NULL;
static PRLock *global_task_lock = NULL;
-static int shutting_down = 0;
+static uint64_t shutting_down = 0;
/***********************************
* Private Defines
@@ -582,7 +582,7 @@ new_task(const char *rawdn, void *plugin)
Slapi_Task *task = NULL;
char *dn = NULL;
- if (rawdn == NULL) {
+ if (rawdn == NULL || shutting_down) {
return NULL;
}
@@ -611,9 +611,20 @@ new_task(const char *rawdn, void *plugin)
PR_Lock(task->task_log_lock);
PR_Lock(global_task_lock);
+ if (shutting_down) {
+ /* Abort! Free everything and return NULL */
+ PR_Unlock(task->task_log_lock);
+ PR_Unlock(global_task_lock);
+ PR_DestroyLock(task->task_log_lock);
+ slapi_ch_free((void **)&task);
+ slapi_ch_free_string(&dn);
+ slapi_log_err(SLAPI_LOG_ERR, "new_task", "Server is shutting down,
aborting task: %s\n", rawdn);
+ return NULL;
+ }
task->next = global_task_list;
global_task_list = task;
PR_Unlock(global_task_lock);
+
task->task_dn = dn;
task->task_state = SLAPI_TASK_SETUP;
task->task_flags = SLAPI_TASK_RUNNING_AS_TASK;
@@ -3004,32 +3015,31 @@ task_init(void)
/* called when the server is shutting down -- abort all existing tasks */
void
-task_shutdown(void)
-{
+task_cancel_all(void) {
Slapi_Task *task;
- int found_any = 0;
- /* first, cancel all tasks */
PR_Lock(global_task_lock);
shutting_down = 1;
for (task = global_task_list; task; task = task->next) {
- if ((task->task_state != SLAPI_TASK_CANCELLED) &&
- (task->task_state != SLAPI_TASK_FINISHED)) {
+ if (task->task_state != SLAPI_TASK_CANCELLED &&
+ task->task_state != SLAPI_TASK_FINISHED)
+ {
task->task_state = SLAPI_TASK_CANCELLED;
if (task->cancel) {
- slapi_log_err(SLAPI_LOG_INFO, "task_shutdown", "Cancelling
task '%s'\n",
+ slapi_log_err(SLAPI_LOG_INFO, "task_cancel_all",
"Canceling task '%s'\n",
task->task_dn);
(*task->cancel)(task);
- found_any = 1;
}
}
}
+ PR_Unlock(global_task_lock);
+}
- if (found_any) {
- /* give any tasks 1 second to say their last rites */
- DS_Sleep(PR_SecondsToInterval(1));
- }
-
+void
+task_shutdown(void)
+{
+ /* Now we can destroy the tasks... */
+ PR_Lock(global_task_lock);
while (global_task_list) {
destroy_task(0, global_task_list);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.