From: Kevin Day Date: Sun, 31 Oct 2021 13:00:17 +0000 (-0500) Subject: Update: Improve sleeping logic, also replacing sleep() with nanosleep(). X-Git-Tag: 0.5.7~99 X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=ad4a95d1b8a26e271d491320b627dbdea443a13c;p=fll Update: Improve sleeping logic, also replacing sleep() with nanosleep(). The previous time-related regression fix introduced unblocking the signals as an immediate solution to its problem. The preferred and intended design is to have the signal handling done by controller rather than done automatically. Redesign the sleep functions to only unblock the signals for the sleep call. Then re-lock the signal so that the intended design may be continued. This then allows all appropriate cleanup functionality to operate as expected. This can be confirmed with valgrind before/after and sending an interrupt during a validation process that utilizes a sleep (such as the example htop rule). Create several functions to standardize some of the processes involved. This also resolves a "todo" designating to replace sleep() with nanosleep(). --- diff --git a/level_3/controller/c/private-common.c b/level_3/controller/c/private-common.c index b5b13d8..832cf55 100644 --- a/level_3/controller/c/private-common.c +++ b/level_3/controller/c/private-common.c @@ -934,6 +934,34 @@ extern "C" { } #endif // _di_controller_time_milliseconds_ +#ifndef _di_controller_time_seconds_ + struct timespec controller_time_seconds(const f_number_unsigned_t seconds) { + + struct timespec time; + time.tv_sec = seconds; + time.tv_nsec = 0; + + return time; + } +#endif // _di_controller_time_seconds_ + +#ifndef _di_controller_time_sleep_nanoseconds_ + int controller_time_sleep_nanoseconds(controller_main_t * const main, controller_setting_t * const setting, struct timespec time) { + + if (setting->interruptible) { + f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0); + } + + const int result = nanosleep(&time, 0); + + if (setting->interruptible) { + f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0); + } + + return result; + } +#endif // _di_controller_time_sleep_nanoseconds_ + #ifdef __cplusplus } // extern "C" #endif diff --git a/level_3/controller/c/private-common.h b/level_3/controller/c/private-common.h index e5d7a36..c47f7f3 100644 --- a/level_3/controller/c/private-common.h +++ b/level_3/controller/c/private-common.h @@ -2477,11 +2477,53 @@ extern "C" { * * @return * A time structure suitable for passing to nanosleep() and similar functions. + * + * @see nanosleep() */ #ifndef _di_controller_time_milliseconds_ extern struct timespec controller_time_milliseconds(const f_number_unsigned_t milliseconds) F_attribute_visibility_internal_d; #endif // _di_controller_time_milliseconds_ +/** + * Convert seconds to nanoseconds. + * + * @param seconds + * The number of seconds. + * + * @return + * A time structure suitable for passing to nanosleep() and similar functions. + * + * @see nanosleep() + */ +#ifndef _di_controller_time_seconds_ + extern struct timespec controller_time_seconds(const f_number_unsigned_t seconds) F_attribute_visibility_internal_d; +#endif // _di_controller_time_seconds_ + +/** + * Sleep for a given number of nanoseconds. + * + * The nanosleep() function handles signals within itself. + * Temporarily unblock signals so that the nanosleep can receive the signal and then restore the signals once done. + * + * The signals will not be unblocked when in uninterruptible mode. + * + * @param main + * The program main data. + * @param setting + * The settings. + * @param time + * The number of nanoseconds to sleep. + * + * @return + * The results of nanosleep(). + * + * @see nanosleep() + * @see controller_time_milliseconds() + */ +#ifndef _di_controller_time_sleep_nanoseconds_ + extern int controller_time_sleep_nanoseconds(controller_main_t * const main, controller_setting_t * const setting, struct timespec time) F_attribute_visibility_internal_d; +#endif // _di_controller_time_sleep_nanoseconds_ + #ifdef __cplusplus } // extern "C" #endif diff --git a/level_3/controller/c/private-rule.c b/level_3/controller/c/private-rule.c index f71eb0f..2654e2e 100644 --- a/level_3/controller/c/private-rule.c +++ b/level_3/controller/c/private-rule.c @@ -1645,9 +1645,9 @@ extern "C" { // sleep for less than a second to better show simulation of synchronous vs asynchronous. { - struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d); + const struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d); - if (nanosleep(&delay, 0) == -1) { + if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) { status = F_signal; } } @@ -1901,9 +1901,9 @@ extern "C" { // sleep for less than a second to better show simulation of synchronous vs asynchronous. { - struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d); + const struct timespec delay = controller_time_milliseconds(controller_thread_simulation_timeout_d); - if (nanosleep(&delay, 0) == -1) { + if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) { status = F_signal; } } @@ -2090,9 +2090,9 @@ extern "C" { } if (rerun_item->delay) { - struct timespec time = controller_time_milliseconds(rerun_item->delay); + const struct timespec delay = controller_time_milliseconds(rerun_item->delay); - if (nanosleep(&time, 0) == -1) { + if (controller_time_sleep_nanoseconds(main, (controller_setting_t *) process->main_setting, delay) == -1) { return -1; } diff --git a/level_3/controller/c/private-thread.c b/level_3/controller/c/private-thread.c index 3812f51..71bc19f 100644 --- a/level_3/controller/c/private-thread.c +++ b/level_3/controller/c/private-thread.c @@ -18,14 +18,13 @@ extern "C" { if (global->thread->enabled != controller_thread_enabled) return 0; - const unsigned int interval = global->main->parameters[controller_parameter_simulate].result == f_console_result_found ? controller_thread_cleanup_interval_short_d : controller_thread_cleanup_interval_long_d; + const struct timespec delay = controller_time_seconds(global->main->parameters[controller_parameter_simulate].result == f_console_result_found ? controller_thread_cleanup_interval_short_d : controller_thread_cleanup_interval_long_d); f_status_t status = F_none; while (global->thread->enabled == controller_thread_enabled) { - // @todo consider switching to nanosleep() which may act better with interrupts and not require f_thread_cancel(). - sleep(interval); + controller_time_sleep_nanoseconds(global->main, global->setting, delay); if (global->thread->enabled != controller_thread_enabled) break; @@ -348,18 +347,6 @@ extern "C" { if (!controller_thread_is_enabled(is_normal, thread)) return; } - // Threads are restricted during the main thread so that the signals can be processed. - // This thread must regain control over the signals within its own thread scope. - { - controller_main_t *main = (controller_main_t *) process->main_data; - controller_setting_t *setting = (controller_setting_t *) process->main_setting; - - if (setting->interruptible) { - f_signal_mask(SIG_UNBLOCK, &main->signal.set, 0); - f_signal_close(&main->signal); - } - } - const f_status_t status = controller_rule_process_do(controller_process_option_asynchronous_d, process); if (status == F_child) { @@ -444,7 +431,7 @@ extern "C" { f_array_length_t j = 0; pid_t pid = 0; - // the sleep() function that is run inside the cleanup function must be interrupted via the f_thread_cancel(). + // The sleep functions that are run inside the cleanup function must be interrupted via the f_thread_cancel(). if (global->thread->id_cleanup) { f_thread_cancel(global->thread->id_cleanup); f_thread_join(global->thread->id_cleanup, 0);