From 6d3fa151d78eecec4d1f576bec66265c94cb7141 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Wed, 14 Apr 2021 17:44:13 -0500 Subject: [PATCH] Update: handle write lock failures, begin adding support for handling read lock failures. Get the response on write lock failure. Present an error message. Return the error status. Increase the cancellation timeouts from 0.06 seconds to 0.6 seconds, making it less aggresive. This results in a 90 second max timeout, which gives more problematic exists a lot more time to cleanly exit. Begin adding support for getting and handing read lock failures. Read locks attempts will be in a loop that checks main thread enabled as well. The timeout is longer than write locks to reduce the CPU overhead as there will be a lot of read locks. Follow up work will utilize this read lock status handling. Remove the additional thread enabled check that follows a write lock. The write lock already checks if the main thread is enabled. --- level_3/controller/c/private-common.c | 65 ++++++++++++- level_3/controller/c/private-common.h | 58 ++++++++++- level_3/controller/c/private-controller.c | 34 ++----- level_3/controller/c/private-rule.c | 156 ++++++++---------------------- level_3/controller/c/private-thread.c | 9 +- 5 files changed, 167 insertions(+), 155 deletions(-) diff --git a/level_3/controller/c/private-common.c b/level_3/controller/c/private-common.c index 870a564..f5f9930 100644 --- a/level_3/controller/c/private-common.c +++ b/level_3/controller/c/private-common.c @@ -171,6 +171,69 @@ extern "C" { } #endif // _di_controller_lock_delete_simple_ +#ifndef _di_controller_lock_error_critical_print_ + void controller_lock_error_critical_print(const fll_error_print_t print, const f_status_t status, const bool read, controller_thread_t *thread) { + + // A signal is not an error. + if (status == F_signal) { + return; + } + + if (print.verbosity != f_console_verbosity_quiet) { + f_thread_mutex_lock(&thread->lock.print); + + fprintf(print.to.stream, "%c", f_string_eol_s[0]); + fprintf(print.to.stream, "%s%sCritical failure while attempting to establish ", print.context.before->string, print.prefix ? print.prefix : f_string_empty_s); + fprintf(print.to.stream, "%s%s%s lock%s", print.context.after->string, print.notable.before->string, read ? "read" : "write", print.notable.after->string); + + if (status != F_failure) { + fprintf(print.to.stream, "%s' due to %s", print.context.before->string, print.context.after->string); + + if (status == F_parameter) { + fprintf(print.to.stream, "%s%s%s%s", print.context.after->string, "Invalid Parameter", print.notable.after->string); + } + else if (status == F_deadlock) { + fprintf(print.to.stream, "%s%s%s%s", print.context.after->string, "Deadlock", print.notable.after->string); + } + else if (status == F_resource_not) { + fprintf(print.to.stream, "%s%s%s%s", print.context.after->string, "Too Many Locks", print.notable.after->string); + } + } + + fprintf(print.to.stream, "%s.%s%c", print.context.before->string, print.context.after->string, f_string_eol_s[0]); + + controller_print_unlock_flush(print.to.stream, &thread->lock.print); + } + } +#endif // _di_controller_lock_error_critical_print_ + +#ifndef _di_controller_lock_read_ + f_status_t controller_lock_read(controller_thread_t * const thread, f_thread_lock_t *lock) { + + struct timespec time; + + f_status_t status = F_none; + + for (;;) { + + controller_time(0, controller_thread_lock_read_timeout, &time); + + status = f_thread_lock_read_timed(&time, lock); + + if (status == F_time) { + if (!thread->enabled) { + return F_signal; + } + } + else { + break; + } + } // for + + return status; + } +#endif // _di_controller_lock_read_ + #ifndef _di_controller_lock_write_ f_status_t controller_lock_write(controller_thread_t * const thread, f_thread_lock_t *lock) { @@ -180,7 +243,7 @@ extern "C" { for (;;) { - controller_time(0, controller_thread_lock_timeout, &time); + controller_time(0, controller_thread_lock_write_timeout, &time); status = f_thread_lock_write_timed(&time, lock); diff --git a/level_3/controller/c/private-common.h b/level_3/controller/c/private-common.h index b796935..5919378 100644 --- a/level_3/controller/c/private-common.h +++ b/level_3/controller/c/private-common.h @@ -1067,11 +1067,15 @@ extern "C" { #ifndef _di_controller_thread_t_ #define controller_thread_cleanup_interval_long 3600 // 1 hour in seconds. #define controller_thread_cleanup_interval_short 180 // 3 minutes in seconds. - #define controller_thread_exit_process_cancel_wait 60000000 // 0.06 seconds in nanoseconds. - #define controller_thread_exit_process_cancel_total 150 // 9 seconds in multiples of wait. - #define controller_thread_lock_timeout 100000000 // 0.1 seconds in nanoseconds. + #define controller_thread_exit_process_cancel_wait 600000000 // 0.6 seconds in nanoseconds. + #define controller_thread_exit_process_cancel_total 150 // 90 seconds in multiples of wait. #define controller_thread_simulation_timeout 200000 // 0.2 seconds in microseconds. + // read locks are more common, use longer waits to reduce the potentially CPU activity. + // write locks are less common, use shorter waits to increase potential response time. + #define controller_thread_lock_read_timeout 2 // seconds + #define controller_thread_lock_write_timeout 100000000 // 0.1 seconds in nanoseconds. + #define controller_thread_wait_timeout_1_before 4 #define controller_thread_wait_timeout_2_before 12 #define controller_thread_wait_timeout_3_before 28 @@ -1347,6 +1351,54 @@ extern "C" { #endif // _di_controller_lock_delete_simple_ /** + * Print a r/w lock related error message, locking the print mutex during the print. + * + * This will ignore F_signal and not print any messages, if passed. + * + * @param print + * Designates how printing is to be performed. + * @param status + * The status code to process. + * Make sure this has F_status_set_fine() called if the status code has any error or warning bits. + * @param read + * If TRUE, then this is for a read lock. + * If FALSE, then this is for a write lock. + * @param thread + * The thread data. + * + * @see fll_error_print() + * @see controller_entry_error_print_cache() + */ +#ifndef _di_controller_lock_error_critical_print_ + extern void controller_lock_error_critical_print(const fll_error_print_t print, const f_status_t status, const bool read, controller_thread_t *thread) f_gcc_attribute_visibility_internal; +#endif // _di_controller_lock_error_critical_print_ + +/** + * Wait to get a read lock. + * + * Given a r/w lock, periodically check to see if main thread is disabled while waiting. + * + * @param lock + * The r/w lock to obtain a read lock on. + * @param thread + * The thread data used to determine if the main thread is disabled or not. + * + * @return + * F_none on success. + * F_signal on (exit) signal received, lock will not be set when this is returned. + * F_status if main thread is disabled and write lock was never achieved. + * + * Status from: f_thread_lock_read_timed(). + * + * Errors (with error bit) from: f_thread_lock_read_timed(). + * + * @see f_thread_lock_read_timed() + */ +#ifndef _di_controller_lock_read_ + extern f_status_t controller_lock_read(controller_thread_t * const thread, f_thread_lock_t *lock) f_gcc_attribute_visibility_internal; +#endif // _di_controller_lock_read_ + +/** * 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/level_3/controller/c/private-controller.c b/level_3/controller/c/private-controller.c index abf6e80..59f6543 100644 --- a/level_3/controller/c/private-controller.c +++ b/level_3/controller/c/private-controller.c @@ -934,13 +934,8 @@ extern "C" { status = controller_lock_write(main->thread, &main->thread->lock.rule); - if (status == F_signal) { - break; - } - - if (!main->thread->enabled) { - f_thread_unlock(&main->thread->lock.rule); - + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main->data->error, F_status_set_fine(status), F_false, main->thread); break; } @@ -1006,12 +1001,8 @@ extern "C" { status = controller_lock_write(main->thread, &main->thread->lock.rule); - if (status == F_signal) { - break; - } - - if (!main->thread->enabled) { - f_thread_unlock(&main->thread->lock.rule); + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main->data->error, F_status_set_fine(status), F_false, main->thread); break; } @@ -1071,12 +1062,8 @@ extern "C" { status = controller_lock_write(main->thread, &main->thread->lock.process); - if (status == F_signal) { - break; - } - - if (!main->thread->enabled) { - f_thread_unlock(&main->thread->lock.process); + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main->data->error, F_status_set_fine(status), F_false, main->thread); break; } @@ -1093,14 +1080,9 @@ extern "C" { status = controller_lock_write(main->thread, &process->lock); - if (status == F_signal) { - f_thread_unlock(&main->thread->lock.process); - - break; - } + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main->data->error, F_status_set_fine(status), F_false, main->thread); - if (!main->thread->enabled) { - f_thread_unlock(&process->lock); f_thread_unlock(&main->thread->lock.process); break; diff --git a/level_3/controller/c/private-rule.c b/level_3/controller/c/private-rule.c index 70ed1e6..cf5ddfa 100644 --- a/level_3/controller/c/private-rule.c +++ b/level_3/controller/c/private-rule.c @@ -996,17 +996,12 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - f_thread_lock_read(&process->lock); - - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); f_thread_lock_read(&process->lock); - return F_signal; + return status_lock; } // assign the child process id to allow for the cancel process to send appropriate termination signals to the child process. @@ -1026,15 +1021,10 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); - f_thread_lock_read(&process->lock); - - return F_signal; + return status_lock; } // remove the pid now that waidpid() has returned. @@ -1155,14 +1145,10 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); - - return F_signal; + return status_lock; } // assign the child process id to allow for the cancel process to send appropriate termination signals to the child process. @@ -1182,14 +1168,10 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); - - return F_signal; + return status_lock; } // remove the pid now that waidpid() has returned. @todo do not clear until forked execution is known to have exited, this is a background execution @@ -1998,17 +1980,12 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - f_thread_lock_read(&process->lock); - - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); f_thread_lock_read(&process->lock); - return F_signal; + return status_lock; } if (F_status_is_error(status)) { @@ -2020,19 +1997,13 @@ extern "C" { status_lock = controller_lock_write(main.thread, &main.thread->lock.rule); - if (status_lock == F_signal) { - f_thread_unlock(&process->lock); - f_thread_lock_read(&process->lock); + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - return F_signal; - } - - if (!main.thread->enabled) { f_thread_unlock(&process->lock); - f_thread_unlock(&main.thread->lock.rule); f_thread_lock_read(&process->lock); - return F_signal; + return status_lock; } if (controller_rule_find(process->rule.alias, main.setting->rules, &id_rule) == F_true) { @@ -2105,19 +2076,13 @@ extern "C" { status = controller_lock_write(main.thread, &process->lock); - if (status == F_signal) { - f_thread_unlock(&process->active); - f_thread_unlock(&main.thread->lock.process); - - return status; - } + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status), F_false, main.thread); - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); f_thread_unlock(&process->active); f_thread_unlock(&main.thread->lock.process); - return F_signal; + return status; } // once a write lock on the process is achieved, it is safe to unlock the main process read lock. @@ -2143,17 +2108,12 @@ extern "C" { status = controller_lock_write(main.thread, &process->lock); - if (status == F_signal) { - f_thread_unlock(&process->active); + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status), F_false, main.thread); - return status; - } - - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); f_thread_unlock(&process->active); - return F_signal; + return status; } process->state = controller_process_state_active; @@ -2287,25 +2247,16 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - f_thread_unlock(&main.thread->lock.rule); - - if (options_force & controller_process_option_asynchronous) { - f_thread_unlock(&process->active); - } - - return status_lock; - } + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (!main.thread->enabled) { f_thread_unlock(&main.thread->lock.rule); - f_thread_unlock(&process->lock); if (options_force & controller_process_option_asynchronous) { f_thread_unlock(&process->active); } - return F_signal; + return status_lock; } controller_rule_delete_simple(&process->rule); @@ -2364,22 +2315,14 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - if (options_force & controller_process_option_asynchronous) { - f_thread_unlock(&process->active); - } - - return status_lock; - } - - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); if (options_force & controller_process_option_asynchronous) { f_thread_unlock(&process->active); } - return F_signal; + return status_lock; } process->stack.array[process->stack.used++] = id_rule; @@ -2421,25 +2364,16 @@ extern "C" { status_lock = controller_lock_write(main.thread, &main.thread->lock.rule); - if (status_lock == F_signal) { - f_thread_unlock(&process->lock); + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); - if (options_force & controller_process_option_asynchronous) { - f_thread_unlock(&process->active); - } - - return status_lock; - } - - if (!main.thread->enabled) { - f_thread_unlock(&main.thread->lock.rule); f_thread_unlock(&process->lock); if (options_force & controller_process_option_asynchronous) { f_thread_unlock(&process->active); } - return F_signal; + return status_lock; } if (controller_rule_find(process->rule.alias, main.setting->rules, &id_rule) == F_true) { @@ -2459,22 +2393,14 @@ extern "C" { status_lock = controller_lock_write(main.thread, &process->lock); - if (status_lock == F_signal) { - if (options_force & controller_process_option_asynchronous) { - f_thread_unlock(&process->active); - } - - return status_lock; - } - - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); + if (status_lock == F_signal || F_status_is_error(status_lock)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status_lock), F_false, main.thread); if (options_force & controller_process_option_asynchronous) { f_thread_unlock(&process->active); } - return F_signal; + return status_lock; } if (options_force & controller_process_option_asynchronous) { @@ -4909,15 +4835,9 @@ extern "C" { status = controller_lock_write(main.thread, &process->lock); - if (status == F_signal) { - f_thread_unlock(&process->active); - f_thread_lock_read(&main.thread->lock.process); + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main.data->error, F_status_set_fine(status), F_false, main.thread); - break; - } - - if (!main.thread->enabled) { - f_thread_unlock(&process->lock); f_thread_unlock(&process->active); f_thread_lock_read(&main.thread->lock.process); diff --git a/level_3/controller/c/private-thread.c b/level_3/controller/c/private-thread.c index e7b769f..b116461 100644 --- a/level_3/controller/c/private-thread.c +++ b/level_3/controller/c/private-thread.c @@ -71,14 +71,9 @@ extern "C" { status = controller_lock_write(main->thread, &process->lock); - if (status == F_signal) { - f_thread_unlock(&process->active); + if (status == F_signal || F_status_is_error(status)) { + controller_lock_error_critical_print(main->data->error, F_status_set_fine(status), F_false, main->thread); - break; - } - - if (!main->thread->enabled) { - f_thread_unlock(&process->lock); f_thread_unlock(&process->active); break; -- 1.8.3.1