From 95015165aa68c4712ee1db9bbade957d0839887b Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Sat, 2 Aug 2025 23:49:12 -0500 Subject: [PATCH] Update: Thread locking and signal tweaks. The `f_thread_condition_signal_all()` requires obtaining the mutex lock before operating. Move this logic into a single function call and repeatedly use it to reduce code repetition. The `controller_lock_signal()` is the function that handles the standard `f_thread_condition_signal_all()` call. Rename `instance.wait' to `instance.wait_condition` to be more consistent. Rename`instance.wait_lock` to `instance.wait` to be more consistent. Add a `controller_lock_mutex()` that operates the same as `controller_lock_read()` but for a mutex lock. The wait condition signal sender uses the newly added `controller_lock_mutex()`. Use an inline static function `private_controller_rule_instance_perform_unlock_active()` to reduce some of the repeated `instance.active` unlocks in the function `controller_rule_instance_perform()`. Change the cleanup thread to only unlock the `instance.lock` and `instance.action` locks when done. I do have concerns with holding the lock before the `f_thread_join()`. I will have to review and test this out some more. --- sources/c/program/controller/init/signal.c | 11 +-- .../program/controller/main/common/type/instance.c | 8 +- .../program/controller/main/common/type/instance.h | 14 ++-- sources/c/program/controller/main/instance.c | 16 ++++ sources/c/program/controller/main/instance.h | 23 ++++++ .../c/program/controller/main/instance/prepare.c | 58 +++++++------- sources/c/program/controller/main/instance/wait.c | 34 ++++---- sources/c/program/controller/main/lock.c | 52 ++++++++++++ sources/c/program/controller/main/lock.h | 89 +++++++++++++++++++++ sources/c/program/controller/main/process.c | 1 + sources/c/program/controller/main/rule/instance.c | 92 +++++++++------------- sources/c/program/controller/main/rule/wait.c | 6 +- sources/c/program/controller/main/thread/cleanup.c | 43 ++++------ sources/c/program/controller/main/thread/entry.c | 11 +-- sources/c/program/controller/main/thread/signal.c | 11 +-- 15 files changed, 295 insertions(+), 174 deletions(-) diff --git a/sources/c/program/controller/init/signal.c b/sources/c/program/controller/init/signal.c index c62687b..7e777a4 100644 --- a/sources/c/program/controller/init/signal.c +++ b/sources/c/program/controller/init/signal.c @@ -30,10 +30,7 @@ extern "C" { // Send F_signal_reserved_35 to cleanup process to reap child processes. if (information.si_signo == F_signal_child || information.si_signo == F_signal_reserved_35) { - if (f_thread_mutex_lock(&main->thread.lock.reap) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.reap_condition); - f_thread_mutex_unlock(&main->thread.lock.reap); - } + controller_lock_signal(&main->thread.lock.reap_condition, &main->thread.lock.reap); continue; } @@ -41,11 +38,7 @@ extern "C" { if (information.si_signo == F_signal_interrupt || information.si_signo == F_signal_abort || information.si_signo == F_signal_quit || information.si_signo == F_signal_termination) { main->thread.signal = information.si_signo; - if (f_thread_mutex_lock_try(&main->thread.lock.reap) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.reap_condition); - f_thread_mutex_unlock(&main->thread.lock.reap); - } - + controller_lock_signal(&main->thread.lock.reap_condition, &main->thread.lock.reap); controller_thread_instance_cancel(main, is_normal, controller_thread_cancel_signal_e); break; diff --git a/sources/c/program/controller/main/common/type/instance.c b/sources/c/program/controller/main/common/type/instance.c index ea5e2a3..5a7ca99 100644 --- a/sources/c/program/controller/main/common/type/instance.c +++ b/sources/c/program/controller/main/common/type/instance.c @@ -16,10 +16,10 @@ extern "C" { instance->id_thread = 0; } - f_thread_condition_delete(&instance->wait); + f_thread_condition_delete(&instance->wait_condition); f_thread_lock_delete(&instance->lock); f_thread_lock_delete(&instance->active); - f_thread_mutex_delete(&instance->wait_lock); + f_thread_mutex_delete(&instance->wait); controller_cache_delete(&instance->cache); controller_rule_delete(&instance->rule); @@ -71,11 +71,11 @@ extern "C" { } if (F_status_is_error_not(status)) { - status = f_thread_condition_create(0, &(*instance)->wait); + status = f_thread_mutex_create(0, &(*instance)->wait); } if (F_status_is_error_not(status)) { - status = f_thread_mutex_create(0, &(*instance)->wait_lock); + status = f_thread_condition_create(0, &(*instance)->wait_condition); } for (f_number_unsigned_t i = 0; i < controller_rule_action_type__enum_size_e; ++i) { diff --git a/sources/c/program/controller/main/common/type/instance.h b/sources/c/program/controller/main/common/type/instance.h index fa3651e..0b44a6e 100644 --- a/sources/c/program/controller/main/common/type/instance.h +++ b/sources/c/program/controller/main/common/type/instance.h @@ -36,10 +36,10 @@ extern "C" { * - type: The currently active process type (from the controller_instance_type_*_e). * - result: The last return code from an execution of a process. * - * - active: A read/write lock representing that something is currently using this (read locks = in use, write lock = begin deleting). - * - lock: A read/write lock on the structure. - * - wait: A thread condition to tell a process waiting process that the Rule has is done being processed. - * - wait_lock: A mutex lock for working with "wait". + * - active: A read/write lock representing that something is currently using this (read locks = in use, write lock = begin deleting). + * - lock: A read/write lock on the structure. + * - wait: A mutex lock for working with "wait". + * - wait_condition: A thread condition to tell a process waiting process that the Rule has is done being processed. * * - child: The process id of a child process, if one is running (when forking to execute a child process). * - path_pids: An array of paths representing PID files. @@ -63,8 +63,8 @@ extern "C" { f_thread_lock_t active; f_thread_lock_t lock; - f_thread_condition_t wait; - f_thread_mutex_t wait_lock; + f_thread_mutex_t wait; + f_thread_condition_t wait_condition; f_pids_t childs; f_string_dynamics_t path_pids; @@ -86,8 +86,8 @@ extern "C" { 0, \ f_thread_lock_t_initialize, \ f_thread_lock_t_initialize, \ - f_thread_condition_t_initialize, \ f_thread_mutex_t_initialize, \ + f_thread_condition_t_initialize, \ f_pids_t_initialize, \ f_string_dynamics_t_initialize, \ f_string_maps_t_initialize, \ diff --git a/sources/c/program/controller/main/instance.c b/sources/c/program/controller/main/instance.c index 7ad9a91..f0b1f51 100644 --- a/sources/c/program/controller/main/instance.c +++ b/sources/c/program/controller/main/instance.c @@ -23,6 +23,22 @@ extern "C" { } #endif // _di_controller_instance_find_ +#ifndef _di_controller_instance_signal_wait_ + f_status_t controller_instance_signal_wait(controller_instance_t * const instance) { + + if (!instance) return F_status_set_error(F_parameter); + + const f_status_t status = controller_lock_mutex(instance->type != controller_instance_type_exit_e, F_true, &instance->main->thread, &instance->wait); + + if (status == F_okay) { + f_thread_condition_signal_all(&instance->wait_condition); + f_thread_mutex_unlock(&instance->wait); + } + + return F_okay; + } +#endif // _di_controller_instance_signal_wait_ + #ifdef __cplusplus } // extern "C" #endif diff --git a/sources/c/program/controller/main/instance.h b/sources/c/program/controller/main/instance.h index 44a5acb..f93771e 100644 --- a/sources/c/program/controller/main/instance.h +++ b/sources/c/program/controller/main/instance.h @@ -41,6 +41,29 @@ extern "C" { f_status_t controller_instance_find(const f_number_unsigned_t action, const f_string_static_t alias, const controller_instances_t instances, f_number_unsigned_t * const at); #endif // _di_controller_instance_find_ +/** + * Send a signal to the wait condition. + * + * Used to inform all things waiting on a specific instance that the specific instance has finished running. + * + * @param instance + * The Controller Instance. + * + * Must not be NULL. + * + * @return + * F_okay on success. + * + * F_parameter (with error bit) if a parameter is invalid. + * + * Errors (with error bit) from: controller_lock_mutex(). + * + * @see controller_lock_mutex() + */ +#ifndef _di_controller_instance_signal_wait_ + extern f_status_t controller_instance_signal_wait(controller_instance_t * const instance); +#endif // _di_controller_instance_signal_wait_ + #ifdef __cplusplus } // extern "C" #endif diff --git a/sources/c/program/controller/main/instance/prepare.c b/sources/c/program/controller/main/instance/prepare.c index 8000df5..f1a2b85 100644 --- a/sources/c/program/controller/main/instance/prepare.c +++ b/sources/c/program/controller/main/instance/prepare.c @@ -21,48 +21,48 @@ extern "C" { } else { status = f_memory_array_increase(controller_allocation_small_d, sizeof(controller_instance_t), (void **) &main->thread.instances.array, &main->thread.instances.used, &main->thread.instances.size); - } - - // The Instances array has instance as a double-pointer. - if (F_status_is_error_not(status) && !main->thread.instances.array[main->thread.instances.used]) { - status = controller_instance_initialize(&main->thread.instances.array[main->thread.instances.used]); - } - - if (F_status_is_error_not(status)) { // The Instances array has instance as a double-pointer. - status = controller_instance_initialize(&main->thread.instances.array[main->thread.instances.used]); - - controller_instance_t * const instance = F_status_is_error_not(status) ? main->thread.instances.array[main->thread.instances.used] : 0; + if (F_status_is_error_not(status) && !main->thread.instances.array[main->thread.instances.used]) { + status = controller_instance_initialize(&main->thread.instances.array[main->thread.instances.used]); + } if (F_status_is_error_not(status)) { - status = controller_lock_write(is_normal, F_true, &main->thread, &instance->lock); - } - if (F_status_is_error(status)) { - controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_false); - } - else { - instance->action = action; - instance->rule.alias.used = 0; - instance->main = main; + // The Instances array has instance as a double-pointer. + status = controller_instance_initialize(&main->thread.instances.array[main->thread.instances.used]); - status = f_string_dynamic_append(alias, &instance->rule.alias); + controller_instance_t * const instance = F_status_is_error_not(status) ? main->thread.instances.array[main->thread.instances.used] : 0; if (F_status_is_error_not(status)) { - instance->id = main->thread.instances.used++; - status = F_okay; + status = controller_lock_write(is_normal, F_true, &main->thread, &instance->lock); + } - if (id) { - *id = instance->id; - } + if (F_status_is_error(status)) { + controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_false); } + else { + instance->action = action; + instance->rule.alias.used = 0; + instance->main = main; + + status = f_string_dynamic_append(alias, &instance->rule.alias); + + if (F_status_is_error_not(status)) { + instance->id = main->thread.instances.used++; + status = F_okay; + + if (id) { + *id = instance->id; + } + } - f_thread_unlock(&instance->lock); + f_thread_unlock(&instance->lock); + } } - } - f_thread_unlock(&main->thread.lock.instance); + f_thread_unlock(&main->thread.lock.instance); + } // The read lock must be restored on return. const f_status_t status_restore = F_status_is_error(controller_lock_read(is_normal, F_false, &main->thread, &main->thread.lock.instance)) diff --git a/sources/c/program/controller/main/instance/wait.c b/sources/c/program/controller/main/instance/wait.c index 938a383..6a7e5a2 100644 --- a/sources/c/program/controller/main/instance/wait.c +++ b/sources/c/program/controller/main/instance/wait.c @@ -18,26 +18,26 @@ extern "C" { uint8_t count = 0; do { - status = f_thread_mutex_lock(&instance->wait_lock); + status = controller_lock_mutex_instance(instance, &instance->wait); + if (!controller_thread_is_enabled_instance(instance)) return F_status_set_error(F_interrupt); + if (F_status_is_error(status)) break; - if (status == F_okay) { - if (count < controller_thread_timeout_wait_1_before_d) { - controller_time_now(controller_thread_timeout_wait_1_seconds_d, controller_thread_timeout_wait_1_nanoseconds_d, &time); - } - else if (count < controller_thread_timeout_wait_2_before_d) { - controller_time_now(controller_thread_timeout_wait_2_seconds_d, controller_thread_timeout_wait_2_nanoseconds_d, &time); - } - else if (count < controller_thread_timeout_wait_3_before_d) { - controller_time_now(controller_thread_timeout_wait_3_seconds_d, controller_thread_timeout_wait_3_nanoseconds_d, &time); - } - else { - controller_time_now(controller_thread_timeout_wait_4_seconds_d, controller_thread_timeout_wait_4_nanoseconds_d, &time); - } + if (count < controller_thread_timeout_wait_1_before_d) { + controller_time_now(controller_thread_timeout_wait_1_seconds_d, controller_thread_timeout_wait_1_nanoseconds_d, &time); + } + else if (count < controller_thread_timeout_wait_2_before_d) { + controller_time_now(controller_thread_timeout_wait_2_seconds_d, controller_thread_timeout_wait_2_nanoseconds_d, &time); + } + else if (count < controller_thread_timeout_wait_3_before_d) { + controller_time_now(controller_thread_timeout_wait_3_seconds_d, controller_thread_timeout_wait_3_nanoseconds_d, &time); + } + else { + controller_time_now(controller_thread_timeout_wait_4_seconds_d, controller_thread_timeout_wait_4_nanoseconds_d, &time); + } - status = f_thread_condition_wait_timed(&time, &instance->wait, &instance->wait_lock); + status = f_thread_condition_wait_timed(&time, &instance->wait_condition, &instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } + f_thread_mutex_unlock(&instance->wait); if (!controller_thread_is_enabled_instance(instance)) return F_status_set_error(F_interrupt); if (F_status_is_error(status)) break; diff --git a/sources/c/program/controller/main/lock.c b/sources/c/program/controller/main/lock.c index ad15289..9bfe2d9 100644 --- a/sources/c/program/controller/main/lock.c +++ b/sources/c/program/controller/main/lock.c @@ -74,6 +74,42 @@ extern "C" { } #endif // _di_controller_lock_create_ +#ifndef _di_controller_lock_mutex_ + f_status_t controller_lock_mutex(const uint8_t is_normal, const uint8_t check, controller_thread_t * const thread, f_thread_mutex_t * const mutex) { + + if (!thread || !mutex) return F_status_set_error(F_parameter); + + f_status_t status = F_okay; + + for (f_time_spec_t time; ; ) { + + memset(&time, 0, sizeof(f_time_spec_t)); + + controller_time_now(controller_thread_timeout_lock_read_seconds_d, controller_thread_timeout_lock_read_nanoseconds_d, &time); + + status = f_thread_mutex_lock_timed(&time, mutex); + + if (status == F_time) { + if (check && !controller_thread_is_enabled(thread, is_normal)) return F_status_set_error(F_interrupt); + } + else { + break; + } + } // for + + return status; + } +#endif // _di_controller_lock_mutex_ + +#ifndef _di_controller_lock_mutex_instance_ + f_status_t controller_lock_mutex_instance(controller_instance_t * const instance, f_thread_mutex_t * const mutex) { + + if (!instance || !mutex) return F_status_set_error(F_parameter); + + return controller_lock_mutex(instance->type != controller_instance_type_exit_e, F_true, &instance->main->thread, mutex); + } +#endif // _di_controller_lock_mutex_instance_ + #ifndef _di_controller_lock_read_ f_status_t controller_lock_read(const uint8_t is_normal, const uint8_t check, controller_thread_t * const thread, f_thread_lock_t * const lock) { @@ -110,6 +146,22 @@ extern "C" { } #endif // _di_controller_lock_read_instance_ +#ifndef _di_controller_lock_signal_ + f_status_t controller_lock_signal(f_thread_condition_t * const condition, f_thread_mutex_t * const mutex) { + + if (!condition || !mutex) return F_status_set_error(F_parameter); + + const f_status_t status = f_thread_mutex_lock(mutex); + + if (status == F_okay) { + f_thread_condition_signal_all(condition); + f_thread_mutex_unlock(mutex); + } + + return F_okay; + } +#endif // _di_controller_lock_signal_ + #ifndef _di_controller_lock_write_ f_status_t controller_lock_write(const uint8_t is_normal, const uint8_t check, controller_thread_t * const thread, f_thread_lock_t * const lock) { diff --git a/sources/c/program/controller/main/lock.h b/sources/c/program/controller/main/lock.h index d06067e..9c3f72c 100644 --- a/sources/c/program/controller/main/lock.h +++ b/sources/c/program/controller/main/lock.h @@ -40,6 +40,69 @@ extern "C" { #endif // _di_controller_lock_create_ /** + * Wait to get a mutex lock. + * + * Given a mutex lock, periodically check to see if main thread is disabled while waiting. + * + * @param is_normal + * If TRUE, then perform as if this operates during a normal operation (Entry and Control). + * If FALSE, then perform as if this operates during a an Exit operation. + * @param check + * If TRUE, then check if the state is enabled and if it is not then abort. + * If FALSE, then do not check if the state is enabled and keep looping. + * @param thread + * The thread data used to determine if the main thread is disabled or not. + * + * Must not be NULL. + * @param mutex + * The mutex to lock. + * + * Must not be NULL. + * + * @return + * F_okay on success. + * + * F_interrupt (with error bit set) on (exit) signal received, lock will not be set when this is returned. + * F_parameter (with error bit) if a parameter is invalid. + * + * Status from: f_thread_mutex_lock_timed(). + * + * Errors (with error bit) from: f_thread_mutex_lock_timed(). + * + * @see f_thread_mutex_lock_timed() + */ +#ifndef _di_controller_lock_mutex_ + extern f_status_t controller_lock_mutex(const uint8_t is_normal, const uint8_t check, controller_thread_t * const thread, f_thread_mutex_t * const mutex); +#endif // _di_controller_lock_mutex_ + +/** + * Wait to get a mutex lock for some instance. + * + * Given a mutex lock, periodically check to see if main thread is disabled while waiting. + * + * @param instance + * The instance to use when checking if thread is enabled. + * + * Must not be NULL. + * @param mutex + * The mutex to lock. + * + * Must not be NULL. + * + * @return + * Status from: controller_lock_mutex(). + * + * F_parameter (with error bit) if a parameter is invalid. + * + * Errors (with error bit) from: controller_lock_mutex(). + * + * @see controller_lock_mutex() + */ +#ifndef _di_controller_lock_mutex_instance_ + extern f_status_t controller_lock_mutex_instance(controller_instance_t * const instance, f_thread_mutex_t * const mutex); +#endif // _di_controller_lock_mutex_instance_ + +/** * Wait to get a read lock. * * Given a r/w lock, periodically check to see if main thread is disabled while waiting. @@ -103,6 +166,32 @@ extern "C" { #endif // _di_controller_lock_read_instance_ /** + * Send a signal to the given condition. + * + * @param condition + * The condition to send the signal too. + * + * Must not be NULL. + * + * @param mutex + * The mutex lock associated with the condition. + * + * Must not be NULL. + * + * @return + * F_okay on success. + * + * F_parameter (with error bit) if a parameter is invalid. + * + * Errors (with error bit) from: controller_lock_mutex(). + * + * @see controller_lock_mutex() + */ +#ifndef _di_controller_lock_signal_ + extern f_status_t controller_lock_signal(f_thread_condition_t * const condition, f_thread_mutex_t * const mutex); +#endif // _di_controller_lock_signal_ + +/** * Wait to get a write lock. * * Given a r/w lock, periodically check to see if main thread is disabled while waiting. diff --git a/sources/c/program/controller/main/process.c b/sources/c/program/controller/main/process.c index cb35290..54cb4ed 100644 --- a/sources/c/program/controller/main/process.c +++ b/sources/c/program/controller/main/process.c @@ -58,6 +58,7 @@ extern "C" { status = f_thread_create(0, &main->thread.id_signal, &controller_thread_signal_normal, (void *) main); if (F_status_is_error_not(status)) { + // Start the clean up thread so that it can handle any zombie reaping. if (!(main->setting.flag & controller_main_flag_validate_d) && main->process.mode == controller_process_mode_service_e) { if (!main->thread.id_cleanup) { diff --git a/sources/c/program/controller/main/rule/instance.c b/sources/c/program/controller/main/rule/instance.c index 12b5f04..a2b5f2c 100644 --- a/sources/c/program/controller/main/rule/instance.c +++ b/sources/c/program/controller/main/rule/instance.c @@ -533,11 +533,7 @@ extern "C" { // The thread is done, so close the thread. if (instance->state == controller_instance_state_done_e) { controller_thread_join(&instance->id_thread); - - if (f_thread_mutex_lock(&instance->wait_lock) == F_okay) { - f_thread_condition_signal_all(&instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } + controller_instance_signal_wait(instance); } instance->id = at; @@ -665,10 +661,7 @@ extern "C" { instance->state = controller_instance_state_idle_e; } - if (f_thread_mutex_lock(&instance->wait_lock) == F_okay) { - f_thread_condition_signal_all(&instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } + controller_instance_signal_wait(instance); f_thread_unlock(&instance->lock); } @@ -680,6 +673,26 @@ extern "C" { #endif // _di_controller_rule_instance_begin_ #ifndef _di_controller_rule_instance_perform_ + /** + * Inline helper function to conditionally unlock instance.active. + * + * @param instance + * The instance data. + * + * Must not be NULL. + * @param options_force + * Force the given instance options, only supporting a subset of instance options. + * + * If controller_instance_option_asynchronous_e, then asynchronously execute. + * If not controller_instance_option_asynchronous_e, then synchronously execute. + */ + static inline void private_controller_rule_instance_perform_unlock_active(controller_instance_t * const instance, const uint8_t options_force) { + + if (options_force & controller_instance_option_asynchronous_e) { + f_thread_unlock(&instance->active); + } + } + f_status_t controller_rule_instance_perform(controller_instance_t * const instance, const uint8_t options_force) { if (!instance || !instance->main) return F_status_set_error(F_parameter); @@ -702,9 +715,7 @@ extern "C" { if (F_status_is_error(status_lock)) { controller_print_error_lock_critical(&instance->main->program.error, F_status_set_fine(status_lock), F_true); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -721,9 +732,7 @@ extern "C" { f_thread_unlock(&instance->lock); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -738,9 +747,7 @@ extern "C" { f_thread_unlock(&instance->main->thread.lock.rule); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -758,9 +765,7 @@ extern "C" { f_thread_unlock(&instance->main->thread.lock.rule); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -775,9 +780,7 @@ extern "C" { // This is a "consider" Action, so do not actually execute the Rule. f_thread_unlock(&instance->lock); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return F_process_not; } @@ -798,9 +801,7 @@ extern "C" { if (!controller_thread_is_enabled_instance(instance)) { f_thread_unlock(&instance->lock); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return F_status_set_error(F_interrupt); } @@ -819,9 +820,7 @@ extern "C" { if (F_status_is_error(status_lock)) { controller_print_error_lock_critical(&instance->main->program.error, F_status_set_fine(status_lock), F_false); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -835,9 +834,7 @@ extern "C" { if (F_status_is_error(status_lock)) { controller_print_error_lock_critical(&instance->main->program.error, F_status_set_fine(status_lock), F_true); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -860,9 +857,7 @@ extern "C" { if (status == F_child) { f_thread_unlock(&instance->lock); - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status; } @@ -876,9 +871,7 @@ extern "C" { f_thread_unlock(&instance->lock); } - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -896,9 +889,7 @@ extern "C" { } if (F_status_set_fine(status) == F_interrupt || F_status_set_fine(status) == F_lock && !controller_thread_is_enabled_instance(instance)) { - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return F_status_set_error(F_interrupt); } @@ -907,10 +898,7 @@ extern "C" { if (F_status_is_error(status_lock)) { controller_print_error_lock_critical(&instance->main->program.error, F_status_set_fine(status_lock), F_false); - - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return status_lock; } @@ -918,17 +906,9 @@ extern "C" { instance->state = (options_force & controller_instance_option_asynchronous_e) ? controller_instance_state_done_e : controller_instance_state_idle_e; instance->stack.used = used_original_stack; - // Inform all things waiting that the instance has finished running. - if (f_thread_mutex_lock(&instance->wait_lock) == F_okay) { - f_thread_condition_signal_all(&instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } - + controller_instance_signal_wait(instance); f_thread_unlock(&instance->lock); - - if (options_force & controller_instance_option_asynchronous_e) { - f_thread_unlock(&instance->active); - } + private_controller_rule_instance_perform_unlock_active(instance, options_force); return controller_thread_is_enabled_instance(instance) ? status : F_status_set_error(F_interrupt); } diff --git a/sources/c/program/controller/main/rule/wait.c b/sources/c/program/controller/main/rule/wait.c index 51de49d..7be4247 100644 --- a/sources/c/program/controller/main/rule/wait.c +++ b/sources/c/program/controller/main/rule/wait.c @@ -106,11 +106,7 @@ extern "C" { instance_list[i]->state = controller_instance_state_idle_e; f_thread_unlock(&instance_list[i]->active); - - if (f_thread_mutex_lock(&instance_list[i]->wait_lock) == F_okay) { - f_thread_condition_signal_all(&instance_list[i]->wait); - f_thread_mutex_unlock(&instance_list[i]->wait_lock); - } + controller_instance_signal_wait(instance_list[i]); } status_lock = controller_lock_read(is_normal, F_true, &main->thread, &instance_list[i]->active); diff --git a/sources/c/program/controller/main/thread/cleanup.c b/sources/c/program/controller/main/thread/cleanup.c index 3f7c1de..108bf99 100644 --- a/sources/c/program/controller/main/thread/cleanup.c +++ b/sources/c/program/controller/main/thread/cleanup.c @@ -51,10 +51,10 @@ extern "C" { // Reaping child processes is not critical, so don't care if the reap flag cannot be unset. if (!main->thread.signal && F_status_is_error_not(status)) { - memset((void *) &signal_data, 0, sizeof(siginfo_t)); reap_count = 0; + do { if (!waitid(P_ALL, 0, &signal_data, WEXITED | WNOHANG)) { if (signal_data.si_pid) { @@ -86,7 +86,7 @@ extern "C" { instance = main->thread.instances.array[i]; - // If "active" has a read lock, then do not attempt to clean it. + // If "active" has a read or write lock, then do not attempt to clean it. if (f_thread_lock_write_try(&instance->active) != F_okay) continue; // If "lock" has a read or write lock, then do not attempt to clean it. @@ -98,8 +98,8 @@ extern "C" { // If instance is active or busy, then do not attempt to clean it. if (instance->state == controller_instance_state_active_e || instance->state == controller_instance_state_busy_e || main->thread.enabled != controller_thread_enabled_e) { - f_thread_unlock(&instance->active); f_thread_unlock(&instance->lock); + f_thread_unlock(&instance->active); if (main->thread.enabled == controller_thread_enabled_e) continue; @@ -118,8 +118,8 @@ extern "C" { } // for if (j < instance->path_pids.used || main->thread.enabled != controller_thread_enabled_e) { - f_thread_unlock(&instance->active); f_thread_unlock(&instance->lock); + f_thread_unlock(&instance->active); if (main->thread.enabled == controller_thread_enabled_e) continue; @@ -127,35 +127,18 @@ extern "C" { } } - f_thread_unlock(&instance->lock); - // Close any still open thread. if (instance->id_thread) { status = f_thread_join(instance->id_thread, 0); if (F_status_is_error_not(status) || F_status_set_fine(status) == F_found_not) { - status = f_thread_lock_write_try(&instance->lock); - - if (status == F_okay) { - instance->state = controller_instance_state_idle_e; - instance->id_thread = 0; - - if (f_thread_mutex_lock(&instance->wait_lock) == F_okay) { - f_thread_condition_signal_all(&instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } + instance->state = controller_instance_state_idle_e; + instance->id_thread = 0; - f_thread_unlock(&instance->lock); - } - else if (F_status_is_error(status)) { - controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_false); - - f_thread_unlock(&instance->active); - - continue; - } + controller_instance_signal_wait(instance); } else { + f_thread_unlock(&instance->lock); f_thread_unlock(&instance->active); continue; @@ -172,11 +155,12 @@ extern "C" { if (instance->childs.array[instance->childs.used]) break; } // for - if (main->thread.enabled != controller_thread_enabled_e) { - f_thread_unlock(&instance->active); + if (main->thread.enabled != controller_thread_enabled_e) { + f_thread_unlock(&instance->lock); + f_thread_unlock(&instance->active); - break; - } + break; + } if (instance->childs.used < instance->childs.size) { f_memory_array_resize(instance->childs.used, sizeof(pid_t), (void **) &instance->childs.array, &instance->childs.used, &instance->childs.size); @@ -195,6 +179,7 @@ extern "C" { controller_rule_delete(&instance->rule); } + f_thread_unlock(&instance->lock); f_thread_unlock(&instance->active); } // for diff --git a/sources/c/program/controller/main/thread/entry.c b/sources/c/program/controller/main/thread/entry.c index adbf53e..c484f68 100644 --- a/sources/c/program/controller/main/thread/entry.c +++ b/sources/c/program/controller/main/thread/entry.c @@ -113,10 +113,7 @@ extern "C" { // According to the manpages, pthread_exit() calls exit(0), which the value of main->program.child should be returned instead. if (main->program.child) exit(main->program.child); } else { - if (f_thread_mutex_lock(&main->thread.lock.alert) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.alert_condition); - f_thread_mutex_unlock(&main->thread.lock.alert); - } + controller_lock_signal(&main->thread.lock.alert_condition, &main->thread.lock.alert); } return 0; @@ -218,11 +215,7 @@ extern "C" { } else { controller_thread_instance_force_set_disable(main); - - if (f_thread_mutex_lock(&main->thread.lock.alert) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.alert_condition); - f_thread_mutex_unlock(&main->thread.lock.alert); - } + controller_lock_signal(&main->thread.lock.alert_condition, &main->thread.lock.alert); } return 0; diff --git a/sources/c/program/controller/main/thread/signal.c b/sources/c/program/controller/main/thread/signal.c index 7caa637..86762b2 100644 --- a/sources/c/program/controller/main/thread/signal.c +++ b/sources/c/program/controller/main/thread/signal.c @@ -30,10 +30,7 @@ extern "C" { // Send F_signal_reserved_35 to cleanup process to reap child processes. if (information.si_signo == F_signal_child || information.si_signo == F_signal_reserved_35) { - if (f_thread_mutex_lock(&main->thread.lock.reap) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.reap_condition); - f_thread_mutex_unlock(&main->thread.lock.reap); - } + controller_lock_signal(&main->thread.lock.reap_condition, &main->thread.lock.reap); continue; } @@ -41,11 +38,7 @@ extern "C" { if (information.si_signo == F_signal_interrupt || information.si_signo == F_signal_abort || information.si_signo == F_signal_quit || information.si_signo == F_signal_termination) { main->thread.signal = information.si_signo; - if (f_thread_mutex_lock_try(&main->thread.lock.reap) == F_okay) { - f_thread_condition_signal_all(&main->thread.lock.reap_condition); - f_thread_mutex_unlock(&main->thread.lock.reap); - } - + controller_lock_signal(&main->thread.lock.reap_condition, &main->thread.lock.reap); controller_thread_instance_cancel(main, is_normal, controller_thread_cancel_signal_e); break; -- 1.8.3.1