From 730c42f34a4e28d14ba1b4903e96630294b1d543 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Fri, 9 Apr 2021 19:15:07 -0500 Subject: [PATCH] Update: lock handling tweaks. Move the rule lock to inside the if condition block because part of the rule is still being read. Add missing unlock for other process active lock before break. Relocate the other process active lock in some cases to be after the last access to the rule. Utilize process r/w locks around starting and joining the main threads. On exit, check to see if id_exit thread is actually used before attempting to join it. --- level_3/controller/c/private-rule.c | 13 ++++++---- level_3/controller/c/private-thread.c | 37 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/level_3/controller/c/private-rule.c b/level_3/controller/c/private-rule.c index e95972277..f9679dc5c 100644 --- a/level_3/controller/c/private-rule.c +++ b/level_3/controller/c/private-rule.c @@ -1760,16 +1760,18 @@ extern "C" { memcpy(alias_other_buffer, main.setting->rules.array[id_rule].alias.string, sizeof(char) * main.setting->rules.array[id_rule].alias.used); alias_other_buffer[main.setting->rules.array[id_rule].alias.used] = 0; - f_thread_unlock(&main.thread->lock.rule); - // attempt to (synchronously) execute the rule when the status is unknown (the rule has not yet been run). if (main.setting->rules.array[id_rule].status == F_known_not) { const f_string_static_t alias_other = f_macro_string_static_t_initialize(alias_other_buffer, main.setting->rules.array[id_rule].alias.used); + f_thread_unlock(&main.thread->lock.rule); + status = controller_rule_process_begin(controller_process_option_execute, alias_other, action, process->options & controller_rule_option_asynchronous ? process->options - controller_rule_option_asynchronous : process->options, process->stack, main, process->cache); if (status == F_child || status == F_signal) { + f_thread_unlock(&process_other->active); + break; } @@ -1799,10 +1801,9 @@ extern "C" { } } } - } - f_thread_lock_read(&main.thread->lock.rule); - f_thread_unlock(&process_other->active); + f_thread_lock_read(&main.thread->lock.rule); + } if (F_status_is_error(main.setting->rules.array[id_rule].status)) { if (i == 0 || i == 1) { @@ -1817,6 +1818,7 @@ extern "C" { if (!(process->options & controller_rule_option_simulate)) { f_thread_unlock(&main.thread->lock.rule); + f_thread_unlock(&process_other->active); break; } } @@ -1833,6 +1835,7 @@ extern "C" { } f_thread_unlock(&main.thread->lock.rule); + f_thread_unlock(&process_other->active); } else { f_thread_unlock(&main.thread->lock.rule); diff --git a/level_3/controller/c/private-thread.c b/level_3/controller/c/private-thread.c index 785652679..4d41bd511 100644 --- a/level_3/controller/c/private-thread.c +++ b/level_3/controller/c/private-thread.c @@ -139,6 +139,8 @@ extern "C" { usleep(controller_thread_exit_main_force_timeout); + f_thread_lock_read(&main->thread->lock.process); + if (main->thread->id_cleanup) { f_thread_signal(main->thread->id_cleanup, F_signal_kill); } @@ -151,6 +153,8 @@ extern "C" { f_thread_signal(main->thread->id_rule, F_signal_kill); } + f_thread_unlock(&main->thread->lock.process); + return 0; } #endif // _di_controller_thread_exit_force_ @@ -180,7 +184,11 @@ extern "C" { } if (F_status_is_error_not(status)) { + f_thread_lock_write(&main.thread->lock.process); + status = f_thread_create(0, &thread.id_signal, &controller_thread_signal, (void *) &main); + + f_thread_unlock(&main.thread->lock.process); } if (F_status_is_error(status)) { @@ -214,9 +222,13 @@ extern "C" { else { const controller_main_entry_t entry = controller_macro_main_entry_t_initialize(&entry_name, &main, setting); + f_thread_lock_write(&main.thread->lock.process); + // the entry processing runs using the rule thread. status = f_thread_create(0, &thread.id_rule, &controller_thread_entry, (void *) &entry); + f_thread_unlock(&main.thread->lock.process); + if (F_status_is_error(status)) { if (data->error.verbosity != f_console_verbosity_quiet) { controller_error_print(data->error, F_status_set_fine(status), "f_thread_create", F_true, &thread); @@ -259,20 +271,32 @@ extern "C" { } if (thread.enabled) { + f_thread_lock_write(&main.thread->lock.process); + status = f_thread_create(0, &thread.id_rule, &controller_thread_rule, (void *) &main); + f_thread_unlock(&main.thread->lock.process); + if (F_status_is_error(status)) { thread.id_rule = 0; } else { + f_thread_lock_write(&main.thread->lock.process); + status = f_thread_create(0, &thread.id_control, &controller_thread_control, (void *) &main); + + f_thread_unlock(&main.thread->lock.process); } if (F_status_is_error(status)) { thread.id_control = 0; } else { + f_thread_lock_write(&main.thread->lock.process); + status = f_thread_create(0, &thread.id_cleanup, &controller_thread_cleanup, (void *) &main); + + f_thread_unlock(&main.thread->lock.process); } if (F_status_is_error(status)) { @@ -302,15 +326,24 @@ extern "C" { } if (data->parameters[controller_parameter_validate].result == f_console_result_none) { + f_thread_lock_read(&main.thread->lock.process); + if (thread.id_signal) { + f_thread_unlock(&main.thread->lock.process); + f_thread_join(thread.id_signal, 0); thread.id_signal = 0; } + else { + f_thread_unlock(&main.thread->lock.process); + } controller_thread_process_cancel(&main); } + f_thread_lock_read(&main.thread->lock.process); + if (thread.id_signal) f_thread_cancel(thread.id_signal); if (thread.id_cleanup) f_thread_cancel(thread.id_cleanup); if (thread.id_control) f_thread_cancel(thread.id_control); @@ -321,8 +354,10 @@ extern "C" { if (thread.id_control) f_thread_join(thread.id_control, 0); if (thread.id_rule) f_thread_join(thread.id_rule, 0); + f_thread_unlock(&main.thread->lock.process); + // wait for exit thread to finish any cleanup. - f_thread_join(thread.id_exit, 0); + if (thread.id_exit) f_thread_join(thread.id_exit, 0); thread.id_cleanup = 0; thread.id_control = 0; -- 2.47.3