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 e959722..f9679dc 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 7856526..4d41bd5 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; -- 1.8.3.1