From 3b31d6b21f71a6f489688a6239084fb4a67c9542 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Wed, 7 Apr 2021 22:25:05 -0500 Subject: [PATCH] Progress: controller program. Update some of the comments. The "consider" was not being loaded into the "process" array. Change the behavior to allow for loading the "consider" rules into the "process" array without executing them. Fix a bug where some entry items were being skipped. The counter was being added an extra time during the loop, resulting in the skip. The child processes are not exiting cleanly. The state F_child was not being fully bubbled up the call stack. It seems that when forking within a thread, the forked child process, on exit will not result in the thread returning to the caller. For now there is 1 block (according to valgrind) that is "possibly lost" for every forked child process. A more in depth investigation is needed. Add a stub for an asynchronous entry that I will be writing for additional testing of the asynchronous processing. --- level_3/controller/c/private-common.h | 16 ++- level_3/controller/c/private-controller.c | 14 ++- level_3/controller/c/private-rule.c | 42 +++++--- level_3/controller/c/private-rule.h | 22 ++-- level_3/controller/c/private-thread.c | 117 ++++++++------------- .../settings/example/entries/asynchronous.entry | 3 + 6 files changed, 102 insertions(+), 112 deletions(-) create mode 100644 level_3/controller/data/settings/example/entries/asynchronous.entry diff --git a/level_3/controller/c/private-common.h b/level_3/controller/c/private-common.h index 2b27685..5789aff 100644 --- a/level_3/controller/c/private-common.h +++ b/level_3/controller/c/private-common.h @@ -772,6 +772,9 @@ extern "C" { * main_thread: Used for passing the controller_thread_t data to the process thread (to populate controller_main_t). */ #ifndef _di_controller_process_t_ + #define controller_process_option_asynchronous 0x1 + #define controller_process_option_execute 0x2 + enum { controller_process_state_idle = 1, controller_process_state_busy, @@ -1113,10 +1116,9 @@ extern "C" { /** * A wrapper used for passing a common set of all data, particularly for sharing between threads. * - * data: All standard program data. - * setting: All loaded settings. - * processs: All Rule Process data. - * thread: All thread related data. + * data: All standard program data. + * setting: All loaded settings. + * thread: All thread related data. */ #ifndef _di_controller_main_t_ typedef struct { @@ -1318,9 +1320,6 @@ extern "C" { /** * Fully deallocate all memory for the given process without caring about return status. * - * This does not close/cancel the id_thread. - * Be sure to properly close and/or detach the thread. - * * @param process * The process to deallocate. * @@ -1349,9 +1348,6 @@ extern "C" { /** * Fully deallocate all memory for the given processs without caring about return status. * - * This does not close/cancel the id_thread for any process. - * Be sure to properly close and/or detach the thread for each process. - * * @param processs * The process array to deallocate. * diff --git a/level_3/controller/c/private-controller.c b/level_3/controller/c/private-controller.c index 726046a..47538fc 100644 --- a/level_3/controller/c/private-controller.c +++ b/level_3/controller/c/private-controller.c @@ -626,6 +626,7 @@ extern "C" { f_array_length_t at_j = 1; uint8_t rule_options = 0; + uint8_t process_options = 0; controller_entry_action_t *entry_action = 0; controller_entry_actions_t *entry_actions = 0; @@ -1078,9 +1079,14 @@ extern "C" { } } - if (F_status_is_error_not(status) && entry_action->type == controller_entry_action_type_rule) { + if (F_status_is_error_not(status)) { + process_options = 0; rule_options = 0; + if (entry_action->type == controller_entry_action_type_rule) { + process_options |= controller_process_option_execute; + } + if (simulate) { rule_options |= controller_rule_option_simulate; } @@ -1094,10 +1100,11 @@ extern "C" { } if (entry_action->code & controller_entry_rule_code_asynchronous) { + process_options |= controller_process_option_asynchronous; rule_options |= controller_rule_option_asynchronous; } - status = controller_rule_process_begin(entry_action->code & controller_entry_rule_code_asynchronous, alias_rule, controller_rule_action_type_start, rule_options, stack, main, *cache); + status = controller_rule_process_begin(process_options, alias_rule, controller_rule_action_type_start, rule_options, stack, main, *cache); if (F_status_set_fine(status) == F_memory_not || status == F_child || status == F_signal) { break; @@ -1225,9 +1232,6 @@ extern "C" { break; } } - else { - cache->ats.array[at_j]++; - } } // for if (status == F_child || status == F_signal) { diff --git a/level_3/controller/c/private-rule.c b/level_3/controller/c/private-rule.c index 80c7323..2b71ee2 100644 --- a/level_3/controller/c/private-rule.c +++ b/level_3/controller/c/private-rule.c @@ -1765,11 +1765,9 @@ extern "C" { const f_string_static_t alias_other = f_macro_string_static_t_initialize(alias_other_buffer, main.setting->rules.array[id_rule].alias.used); - status = controller_rule_process_begin(F_false, alias_other, action, process->options & controller_rule_option_asynchronous ? process->options - controller_rule_option_asynchronous : process->options, process->stack, main, process->cache); + 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; } @@ -1951,7 +1949,7 @@ extern "C" { #endif // _di_controller_rule_process_ #ifndef _di_controller_rule_process_begin_ - f_status_t controller_rule_process_begin(const bool asynchronous, const f_string_static_t alias_rule, const uint8_t action, const uint8_t options, const f_array_lengths_t stack, const controller_main_t main, const controller_cache_t cache) { + f_status_t controller_rule_process_begin(const uint8_t process_options, const f_string_static_t alias_rule, const uint8_t action, const uint8_t options, const f_array_lengths_t stack, const controller_main_t main, const controller_cache_t cache) { if (!main.thread->enabled) { return F_signal; @@ -2079,7 +2077,7 @@ extern "C" { f_thread_unlock(&process->lock); if (F_status_is_error_not(status)) { - if (asynchronous) { + if ((process_options & controller_process_option_execute) && (process_options & controller_process_option_asynchronous)) { status = f_thread_create(0, &process->id_thread, controller_thread_process, (void *) process); if (F_status_is_error(status)) { @@ -2087,7 +2085,11 @@ extern "C" { } } else { - status = controller_rule_process_do(F_false, process); + status = controller_rule_process_do(process_options, process); + + if (status == F_child || status == F_signal) { + return status; + } } } @@ -2102,17 +2104,17 @@ extern "C" { #endif // _di_controller_rule_process_begin_ #ifndef _di_controller_rule_process_do_ - f_status_t controller_rule_process_do(const bool asynchronous, controller_process_t *process) { + f_status_t controller_rule_process_do(const uint8_t options, controller_process_t *process) { // the process lock shall be held for the duration of this processing (aside from switching between read to/from write). - if (asynchronous) f_thread_lock_read(&process->active); + if (options & controller_process_option_asynchronous) f_thread_lock_read(&process->active); f_thread_lock_read(&process->lock); controller_main_t main = controller_macro_main_t_initialize((controller_data_t *) process->main_data, (controller_setting_t *) process->main_setting, (controller_thread_t *) process->main_thread); if (!main.thread->enabled) { f_thread_unlock(&process->lock); - if (asynchronous) f_thread_unlock(&process->active); + if (options & controller_process_option_asynchronous) f_thread_unlock(&process->active); return F_signal; } @@ -2141,7 +2143,7 @@ extern "C" { if (F_status_is_error(status)) { controller_entry_error_print(main.data->error, process->cache.action, F_status_set_fine(status), "controller_rule_copy", F_true, main.thread); } - else { + else if (options & controller_process_option_execute) { for (f_array_length_t i = 0; i < process->stack.used && main.thread->enabled; ++i) { if (process->stack.array[i] == id_rule) { @@ -2193,7 +2195,7 @@ extern "C" { f_thread_unlock(&main.thread->lock.rule); } - else { + else if (options & controller_process_option_execute) { if (main.thread->enabled) { status = controller_rule_process(controller_rule_action_type_start, main, process); } @@ -2222,10 +2224,14 @@ extern "C" { } } + if (status == F_child || status == F_signal) { + return status; + } + f_thread_unlock(&process->lock); f_thread_lock_write(&process->lock); - if (asynchronous) { + if ((options & controller_process_option_execute) && (options & controller_process_option_asynchronous)) { process->state = controller_process_state_done; } else { @@ -2235,14 +2241,16 @@ extern "C" { process->stack.used = used_original_stack; f_thread_unlock(&process->lock); - if (asynchronous) f_thread_unlock(&process->active); + if (options & controller_process_option_asynchronous) f_thread_unlock(&process->active); if (main.thread->enabled) { + if (options & controller_process_option_execute) { - // inform all things waiting that the process has finished running. - f_thread_mutex_lock(&process->wait_lock); - f_thread_condition_signal_all(&process->wait); - f_thread_mutex_unlock(&process->wait_lock); + // inform all things waiting that the process has finished running. + f_thread_mutex_lock(&process->wait_lock); + f_thread_condition_signal_all(&process->wait); + f_thread_mutex_unlock(&process->wait_lock); + } } else { return F_signal; diff --git a/level_3/controller/c/private-rule.h b/level_3/controller/c/private-rule.h index 0ec6434..e6a32f7 100644 --- a/level_3/controller/c/private-rule.h +++ b/level_3/controller/c/private-rule.h @@ -592,9 +592,12 @@ extern "C" { /** * Synchronously or Asynchronously begin processing some rule. * - * @param asynchronous - * If TRUE, then asynchronously execute a process. - * If FALSE, then synchronously execute a process. + * @param process_options + * If controller_process_option_asynchronous, then asynchronously execute. + * If controller_process_option_execute, then load and execute. + * + * If not controller_process_option_asynchronous, then synchronously execute. + * If not controller_process_option_execute, then load only, do not execute at all. * @param alias_rule * The alias of the rule, such as "boot/init". * @param action @@ -628,7 +631,7 @@ extern "C" { * @see f_thread_create() */ #ifndef _di_controller_rule_process_begin_ - extern f_status_t controller_rule_process_begin(const bool asynchronous, const f_string_static_t alias_rule, const uint8_t action, const uint8_t options, const f_array_lengths_t stack, const controller_main_t main, const controller_cache_t cache) f_gcc_attribute_visibility_internal; + extern f_status_t controller_rule_process_begin(const uint8_t process_options, const f_string_static_t alias_rule, const uint8_t action, const uint8_t options, const f_array_lengths_t stack, const controller_main_t main, const controller_cache_t cache) f_gcc_attribute_visibility_internal; #endif // _di_controller_rule_process_begin_ /** @@ -637,9 +640,12 @@ extern "C" { * This does all the preparation work that needs to be synchronously performed within the same thread. * This will copy the rule by the alias to the process structure. * - * @param asynchronous - * If TRUE, designates that this function is being asynchronously executed. - * If FALSE, designates that this function is being synchronously executed. + * @param options + * If controller_process_option_asynchronous, then asynchronously execute. + * If controller_process_option_execute, then load and execute. + * + * If not controller_process_option_asynchronous, then synchronously execute. + * If not controller_process_option_execute, then load only, do not execute at all. * @param process * The process data. * @@ -660,7 +666,7 @@ extern "C" { * @see controller_rule_process_begin() */ #ifndef _di_controller_rule_process_do_ - extern f_status_t controller_rule_process_do(const bool asynchronous, controller_process_t *process) f_gcc_attribute_visibility_internal; + extern f_status_t controller_rule_process_do(const uint8_t options, controller_process_t *process) f_gcc_attribute_visibility_internal; #endif // _di_controller_rule_process_do_ /** diff --git a/level_3/controller/c/private-thread.c b/level_3/controller/c/private-thread.c index 8699528..6dd8f18 100644 --- a/level_3/controller/c/private-thread.c +++ b/level_3/controller/c/private-thread.c @@ -14,7 +14,7 @@ extern "C" { const controller_main_t *main = (controller_main_t *) arguments; - if (!main->thread->enabled) f_thread_exit(0); + if (!main->thread->enabled) return 0; const unsigned int interval = main->data->parameters[controller_parameter_test].result == f_console_result_found ? controller_thread_cleanup_interval_short : controller_thread_cleanup_interval_long; @@ -55,6 +55,7 @@ extern "C" { f_thread_unlock(&process->lock); + // close any still open thread. if (process->id_thread) { f_thread_join(process->id_thread, 0); @@ -79,69 +80,11 @@ extern "C" { f_thread_unlock(&process->active); } // for - if (main->thread->processs.size) { - f_array_length_t j = main->thread->processs.size; - - for (i = main->thread->processs.size - 1; j && main->thread->enabled; --i, --j) { - - if (!main->thread->processs.array[i]) continue; - - process = main->thread->processs.array[i]; - - // if "active" has a read lock, then do not attempt to clean it. - if (f_thread_lock_write_try(&process->active) != F_none) { - break; - } - - // if "lock" has a read or write lock, then do not attempt to clean it. - if (f_thread_lock_write_try(&process->lock) != F_none) { - f_thread_unlock(&process->active); - - break; - } - - // if process is active or busy, then do not attempt to clean it. - if (process->state == controller_process_state_active || process->state == controller_process_state_busy) { - f_thread_unlock(&process->active); - f_thread_unlock(&process->lock); - - break; - } - - f_thread_unlock(&process->lock); - - if (process->id_thread) { - f_thread_join(process->id_thread, 0); - - if (!main->thread->enabled) { - f_thread_unlock(&process->active); - - break; - } - - f_thread_lock_write(&process->lock); - - process->state = controller_process_state_idle; - process->id_thread = 0; - - f_thread_unlock(&process->lock); - } - - // deallocate dynamic portions of the structure that are only ever needed while the rule is being processed. - controller_cache_delete_simple(&process->cache); - f_type_array_lengths_resize(0, &process->stack); - - --main->thread->processs.used; - - f_thread_unlock(&process->active); - } // for - } - f_thread_unlock(&main->thread->lock.process); } } // while - f_thread_exit(0); + return 0; } #endif // _di_controller_thread_cleanup_ @@ -150,9 +93,9 @@ extern "C" { controller_main_t *main = (controller_main_t *) arguments; - if (!main->thread->enabled) f_thread_exit(0); + if (!main->thread->enabled) return 0; - f_thread_exit(0); + return 0; } #endif // _di_controller_thread_control_ @@ -203,7 +146,7 @@ extern "C" { f_thread_signal(main->thread->id_rule, F_signal_kill); } - f_thread_exit(0); + return 0; } #endif // _di_controller_thread_exit_force_ @@ -297,6 +240,12 @@ extern "C" { // wait for the entry thread to complete before starting the rule thread. f_thread_join(thread.id_rule, 0); + if (thread.status == F_child) { + controller_thread_delete_simple(&thread); + + return F_child; + } + thread.id_rule = 0; if (thread.enabled) { @@ -388,12 +337,25 @@ extern "C" { { controller_thread_t *thread = (controller_thread_t *) process->main_thread; - if (!thread->enabled) f_thread_exit(0); + if (!thread->enabled) return 0; } - controller_rule_process_do(F_true, process); + if (controller_rule_process_do(controller_process_option_asynchronous | controller_process_option_execute, process) == F_child) { + + // A forked child process should deallocate memory on exit. + // It seems that this function doesn't return to the calling thread for a forked child process, even with the "return 0;" below. + // Deallocate as much as possible. - f_thread_exit(0); + controller_data_t *data = (controller_data_t *) process->main_data; + controller_setting_t *setting = (controller_setting_t *) process->main_setting; + controller_thread_t *thread = (controller_thread_t *) process->main_thread; + + controller_thread_delete_simple(thread); + controller_setting_delete_simple(setting); + controller_delete_data(data); + } + + return 0; } #endif // _di_controller_thread_process_ @@ -479,7 +441,7 @@ extern "C" { controller_main_entry_t *entry = (controller_main_entry_t *) arguments; - if (!entry->main->thread->enabled) f_thread_exit(0); + if (!entry->main->thread->enabled) return 0; controller_data_t *data = entry->main->data; controller_cache_t *cache = &entry->main->thread->cache; @@ -529,7 +491,18 @@ extern "C" { } } - f_thread_exit(0); + if (*status == F_child) { + + // A forked child process should deallocate memory on exit. + // It seems that this function doesn't return to the calling thread for a forked child process, even with the "return 0;" below. + // Deallocate as much as possible. + + controller_thread_delete_simple(entry->main->thread); + controller_setting_delete_simple(entry->main->setting); + controller_delete_data(entry->main->data); + } + + return 0; } #endif // _di_controller_thread_entry_ @@ -538,9 +511,9 @@ extern "C" { controller_main_t *main = (controller_main_t *) arguments; - if (!main->thread->enabled) f_thread_exit(0); + if (!main->thread->enabled) return 0; - f_thread_exit(0); + return 0; } #endif // _di_controller_thread_rule_ @@ -549,7 +522,7 @@ extern "C" { controller_main_t *main = (controller_main_t *) arguments; - if (!main->thread->enabled) f_thread_exit(0); + if (!main->thread->enabled) return 0; for (int signal = 0; main->thread->enabled; ) { @@ -565,7 +538,7 @@ extern "C" { } } // for - f_thread_exit(0); + return 0; } #endif // _di_controller_thread_signal_ diff --git a/level_3/controller/data/settings/example/entries/asynchronous.entry b/level_3/controller/data/settings/example/entries/asynchronous.entry new file mode 100644 index 0000000..67f0d67 --- /dev/null +++ b/level_3/controller/data/settings/example/entries/asynchronous.entry @@ -0,0 +1,3 @@ +# fss-0005 + +main: -- 1.8.3.1