From: Kevin Day Date: Sat, 6 Jul 2024 23:58:47 +0000 (-0500) Subject: Progress: Continue migrating the project, addressing instances double-pointer. X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=3f97e9398074dc2c3b47118ba540ad9e9ccda7f2;p=controller Progress: Continue migrating the project, addressing instances double-pointer. I forgot that instances is a double-pointer. Adjust the code to ensure that the double-pointer is handled and that the locks are allocated and de-allocated. Add additional comments regarding things that need to be done. --- diff --git a/sources/c/main/common/type/instance.c b/sources/c/main/common/type/instance.c index 1a837a3..5d95132 100644 --- a/sources/c/main/common/type/instance.c +++ b/sources/c/main/common/type/instance.c @@ -9,24 +9,83 @@ extern "C" { if (!instance) return; + if (instance->id_thread) { + f_thread_signal_write(instance->id_thread, F_signal_kill); + f_thread_join(instance->id_thread, 0); + + instance->id_thread = 0; + } + + // TODO: Review how the project uses locks and how to safely delete them. + // The POSIX standard might quite horribly provide undefined behavior on double de-allocaton without providing a way to tell if the data is already de-allocated or not. + // There may need to be a boolean/flag on each instance to designate whether or not the locks were initialized or not. + // Actually, the instances is a double-pointer so that double-point can act as a boolean/flag. + f_thread_condition_delete(&instance->wait); + f_thread_lock_delete(&instance->lock); + f_thread_lock_delete(&instance->active); + f_thread_mutex_delete(&instance->wait_lock); + controller_cache_delete(&instance->cache); + controller_rule_delete(&instance->rule); + + f_memory_array_resize(0, sizeof(pid_t), (void **) &instance->childs.array, &instance->childs.used, &instance->childs.size); + f_memory_array_resize(0, sizeof(f_number_unsigned_t), (void **) &instance->stack.array, &instance->stack.used, &instance->stack.size); + + f_memory_arrays_resize(0, sizeof(f_string_dynamic_t), (void **) &instance->path_pids.array, &instance->path_pids.used, &instance->path_pids.size, &f_string_dynamics_delete_callback); } #endif // _di_controller_instance_delete_ -#ifndef _di_f_instances_delete_callback_ - f_status_t f_instances_delete_callback(const f_number_unsigned_t start, const f_number_unsigned_t stop, void * const void_array) { +#ifndef _di_controller_instances_delete_callback_ + f_status_t controller_instances_delete_callback(const f_number_unsigned_t start, const f_number_unsigned_t stop, void * const void_array) { { - controller_instance_t * const instances = (controller_instance_t *) void_array; + controller_instance_t ** const instances = (controller_instance_t **) void_array; for (f_number_unsigned_t i = start; i < stop; ++i) { - controller_instance_delete(&instances[i]); + + if (instances[i]) { + controller_instance_delete(instances[i]); + + instances[i] = 0; + } } // for } return F_okay; } -#endif // _di_f_instances_delete_callback_ +#endif // _di_controller_instances_delete_callback_ + +#ifndef _di_controller_instance_initialize_ + f_status_t controller_instance_initialize(controller_instance_t ** restrict const instance) { + + f_status_t status = f_memory_new(1, sizeof(controller_instance_t), (void **) instance); + + if (F_status_is_error_not(status)) { + status = f_thread_lock_create(0, &(*instance)->lock); + } + + if (F_status_is_error_not(status)) { + status = f_thread_lock_create(0, &(*instance)->active); + } + + if (F_status_is_error_not(status)) { + status = f_thread_condition_create(0, &(*instance)->wait); + } + + if (F_status_is_error_not(status)) { + status = f_thread_mutex_create(0, &(*instance)->wait_lock); + } + + for (f_number_unsigned_t i = 0; i < controller_rule_action_type__enum_size_e; ++i) { + (*instance)->rule.status[i] = F_known_not; + } // for + + // TODO: There probably should be a boolean/flag to represent that the locks were initialized. + // And on allocation failure on any lock before all locks are allocated should require that any allocated locks be de-allocated before returning on error. + + return F_status_is_error(status) ? status : F_okay; + } +#endif // _di_controller_instance_initialize_ #ifdef __cplusplus } // extern "C" diff --git a/sources/c/main/common/type/instance.h b/sources/c/main/common/type/instance.h index 2370097..bfdac65 100644 --- a/sources/c/main/common/type/instance.h +++ b/sources/c/main/common/type/instance.h @@ -100,8 +100,10 @@ extern "C" { * * The typedef for this is located in the defs.h header. * + * Instances in this array are pointers and must each be individually allocated. + * * Properties: - * - array: An array of Instances. + * - array: An array of Instances pointers (note the double-pointer). * - size: Total amount of allocated space. * - used: Total number of allocated spaces used. */ @@ -157,9 +159,34 @@ extern "C" { * * @see controller_instance_delete() */ -#ifndef _di_f_instances_delete_callback_ - extern f_status_t f_instances_delete_callback(const f_number_unsigned_t start, const f_number_unsigned_t stop, void * const array); -#endif // _di_f_instances_delete_callback_ +#ifndef _di_controller_instances_delete_callback_ + extern f_status_t controller_instances_delete_callback(const f_number_unsigned_t start, const f_number_unsigned_t stop, void * const array); +#endif // _di_controller_instances_delete_callback_ + +/** + * Perform initialization that must be performed exactly once for each Instance. + * + * This performs the initial lock allocation as well as other such defaults. + * + * @param instance + * The Instance to initialize. + * + * Must not be NULL. + * + * @return + * F_okay on success. + * + * Errors (with error bit) from: f_thread_condition_create(). + * Errors (with error bit) from: f_thread_lock_create(). + * Errors (with error bit) from: f_thread_mutex_create(). + * + * @see f_thread_condition_create() + * @see f_thread_lock_create() + * @see f_thread_mutex_create() + */ +#ifndef _di_controller_instance_initialize_ + extern f_status_t controller_instance_initialize(controller_instance_t ** restrict const instance); +#endif // _di_controller_instance_initialize_ #ifdef __cplusplus } // extern "C" diff --git a/sources/c/main/common/type/thread.c b/sources/c/main/common/type/thread.c index 1f497d2..025b34f 100644 --- a/sources/c/main/common/type/thread.c +++ b/sources/c/main/common/type/thread.c @@ -12,7 +12,7 @@ extern "C" { controller_lock_delete(&thread->lock); controller_cache_delete(&thread->cache); - f_memory_arrays_resize(0, sizeof(controller_instance_t), (void **) &thread->instances.array, &thread->instances.used, &thread->instances.size, &f_instances_delete_callback); + f_memory_arrays_resize(0, sizeof(controller_instance_t), (void **) &thread->instances.array, &thread->instances.used, &thread->instances.size, &controller_instances_delete_callback); } #endif // _di_controller_thread_delete_ diff --git a/sources/c/main/instance/prepare.c b/sources/c/main/instance/prepare.c index 29f4e16..39450ec 100644 --- a/sources/c/main/instance/prepare.c +++ b/sources/c/main/instance/prepare.c @@ -5,7 +5,7 @@ extern "C" { #endif #ifndef _di_controller_instance_prepare_ - f_status_t controller_instance_prepare(controller_t * const main, const uint8_t is_normal, const uint8_t action, const f_string_static_t alias, f_number_unsigned_t *id) { + f_status_t controller_instance_prepare(controller_t * const main, const uint8_t is_normal, const uint8_t action, const f_string_static_t alias, f_number_unsigned_t * const id) { if (!main) return F_status_set_error(F_parameter); @@ -23,11 +23,26 @@ extern "C" { 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); } - if (F_status_is_error_not(status) && main->thread.instances.array[main->thread.instances.used]) { + printf("\nDEBUG: instances = (%lu, %lu), status=%d, %lu\n", main->thread.instances.used, main->thread.instances.size, F_status_set_fine(status), main->thread.instances.array[main->thread.instances.used]); - controller_instance_t * const instance = main->thread.instances.array[main->thread.instances.used]; + // 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)) { + printf("\nDEBUG: instances before initialization = (%lu, %lu)\n", main->thread.instances.used, main->thread.instances.size); + + // The Instances array has instance as a double-pointer. + status = controller_instance_initialize(&main->thread.instances.array[main->thread.instances.used]); + + printf("\nDEBUG: initialized instance at %lu, status = %d\n", main->thread.instances.used, F_status_set_fine(status)); - status = controller_lock_write(is_normal, &main->thread, &instance->lock); + 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)) { + status = controller_lock_write(is_normal, &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); @@ -55,9 +70,13 @@ extern "C" { f_thread_unlock(&main->thread.lock.instance); // The read lock must be restored on return. - status = F_status_is_error(controller_lock_read(is_normal, &main->thread, &main->thread.lock.instance)) + const f_status_t status_restore = F_status_is_error(controller_lock_read(is_normal, &main->thread, &main->thread.lock.instance)) ? F_status_set_error(F_lock) : F_okay; + + if (F_status_is_fine(status)) { + status = status_restore; + } } else { status = F_found; diff --git a/sources/c/main/instance/prepare.h b/sources/c/main/instance/prepare.h index 8e400ce..bd75376 100644 --- a/sources/c/main/instance/prepare.h +++ b/sources/c/main/instance/prepare.h @@ -95,7 +95,7 @@ extern "C" { * @see controller_instance_prepare() */ #ifndef _di_controller_instance_prepare_instance_type_ - extern f_status_t controller_instance_prepare_instance_type(controller_t * const main, const uint8_t type, const uint8_t action, const f_string_static_t alias, f_number_unsigned_t *id); + extern f_status_t controller_instance_prepare_instance_type(controller_t * const main, const uint8_t type, const uint8_t action, const f_string_static_t alias, f_number_unsigned_t * const id); #endif // _di_controller_instance_prepare_instance_type_ #ifdef __cplusplus diff --git a/sources/c/main/lock.c b/sources/c/main/lock.c index e9b556e..ad3c93a 100644 --- a/sources/c/main/lock.c +++ b/sources/c/main/lock.c @@ -48,6 +48,8 @@ extern "C" { for (f_time_spec_t time; ; ) { + memset(&time, 0, sizeof(f_time_spec_t)); + controller_time_now(controller_thread_lock_read_timeout_seconds_d, controller_thread_lock_read_timeout_nanoseconds_d, &time); status = f_thread_lock_read_timed(&time, lock); diff --git a/sources/c/main/process.c b/sources/c/main/process.c index b5bc2de..42a8f1b 100644 --- a/sources/c/main/process.c +++ b/sources/c/main/process.c @@ -30,16 +30,16 @@ extern "C" { if (F_status_is_error(status)) { controller_print_error_status(&main->program.error, macro_controller_f(controller_lock_create), status); - } else { + }/* else { // TODO: Is this block needed here, given allocation will also happen later on as needed? 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); if (F_status_is_error(status)) { controller_print_error_status(&main->program.error, macro_controller_f(f_memory_array_increase), status); } - } + }*/ if (F_status_is_error_not(status)) { - status = f_thread_create(0, &main->thread.id_signal, &controller_thread_signal_normal, (void *) &main); + status = f_thread_create(0, &main->thread.id_signal, &controller_thread_signal_normal, (void *) main); } if (F_status_is_error(status)) { @@ -59,7 +59,7 @@ extern "C" { } } else if (main->process.name_entry.used) { - status = f_thread_create(0, &main->thread.id_entry, &controller_thread_entry, (void *) &main); + status = f_thread_create(0, &main->thread.id_entry, &controller_thread_entry, (void *) main); if (F_status_is_error(status)) { controller_print_error_status(&main->program.error, macro_controller_f(f_thread_create), status); @@ -81,13 +81,13 @@ extern "C" { controller_thread_join(&main->thread.id_rule); if (main->thread.enabled && main->process.mode == controller_process_mode_service_e) { - status = f_thread_create(0, &main->thread.id_rule, &controller_thread_rule, (void *) &main); + status = f_thread_create(0, &main->thread.id_rule, &controller_thread_rule, (void *) main); if (F_status_is_error(status)) { main->thread.id_rule = 0; } else { - status = f_thread_create(0, &main->thread.id_cleanup, &controller_thread_cleanup, (void *) &main); + status = f_thread_create(0, &main->thread.id_cleanup, &controller_thread_cleanup, (void *) main); } if (F_status_is_error(status)) { diff --git a/sources/c/main/rule/instance.c b/sources/c/main/rule/instance.c index 59cbb1f..a605449 100644 --- a/sources/c/main/rule/instance.c +++ b/sources/c/main/rule/instance.c @@ -550,12 +550,7 @@ extern "C" { return F_status_set_error(F_interrupt); } - f_status_t status = F_okay; - f_status_t status_lock = F_okay; - - controller_instance_t *instance = 0; - - status = controller_lock_read_instance_type(type, &main->thread, &main->thread.lock.instance); + f_status_t status = controller_lock_read_instance_type(type, &main->thread, &main->thread.lock.instance); if (F_status_is_error(status)) { controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_true); @@ -563,73 +558,71 @@ extern "C" { return status; } - { - f_number_unsigned_t at = 0; - - status = controller_instance_prepare(main, type != controller_instance_type_exit_e, action, alias_rule, &at); + f_number_unsigned_t at = 0; - if (F_status_is_error(status)) { - f_thread_unlock(&main->thread.lock.instance); + status = controller_instance_prepare(main, type != controller_instance_type_exit_e, action, alias_rule, &at); - if (main->program.error.verbosity > f_console_verbosity_quiet_e) { - controller_lock_print(main->program.error.to, &main->thread); + if (F_status_is_error(status)) { + f_thread_unlock(&main->thread.lock.instance); - controller_print_error_rule_item_rule_not_loaded(&main->program.error, alias_rule); - controller_print_error_rule_cache(&main->program.error, &cache->action, F_false); + if (main->program.error.verbosity > f_console_verbosity_quiet_e) { + controller_lock_print(main->program.error.to, &main->thread); - controller_unlock_print_flush(main->program.error.to, &main->thread); - } + controller_print_error_rule_item_rule_not_loaded(&main->program.error, alias_rule); + controller_print_error_rule_cache(&main->program.error, &cache->action, F_false); - return status; + controller_unlock_print_flush(main->program.error.to, &main->thread); } - instance = main->thread.instances.array[at]; - - status = controller_lock_read_instance_type(type, &main->thread, &instance->active); + return status; + } - if (F_status_is_error(status)) { - controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_true); - controller_print_error_rule_item(&main->program.error, &cache->action, F_false, F_status_set_fine(status)); + controller_instance_t * const instance = main->thread.instances.array[at]; - f_thread_unlock(&main->thread.lock.instance); + status = controller_lock_read_instance_type(type, &main->thread, &instance->active); - return status; - } + if (F_status_is_error(status)) { + controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status), F_true); + controller_print_error_rule_item(&main->program.error, &cache->action, F_false, F_status_set_fine(status)); - status_lock = controller_lock_write_instance(instance, &instance->lock); + f_thread_unlock(&main->thread.lock.instance); - if (F_status_is_error(status_lock)) { - controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status_lock), F_false); + return status; + } - f_thread_unlock(&instance->active); - f_thread_unlock(&main->thread.lock.instance); + f_status_t status_lock = controller_lock_write_instance(instance, &instance->lock); - return status_lock; - } + if (F_status_is_error(status_lock)) { + controller_print_error_lock_critical(&main->program.error, F_status_set_fine(status_lock), F_false); - // Once a write lock on the instance is achieved, it is safe to unlock the instance read lock. + f_thread_unlock(&instance->active); f_thread_unlock(&main->thread.lock.instance); - // If the instance is already running, then there is nothing to do. - if (instance->state == controller_instance_state_active_e || instance->state == controller_instance_state_busy_e) { - f_thread_unlock(&instance->lock); - f_thread_unlock(&instance->active); + return status_lock; + } - return F_busy; - } + // Once a write lock on the instance is achieved, it is safe to unlock the instance read lock. + f_thread_unlock(&main->thread.lock.instance); - // The thread is done, so close the thread. - if (instance->state == controller_instance_state_done_e) { - controller_thread_join(&instance->id_thread); + // If the instance is already running, then there is nothing to do. + if (instance->state == controller_instance_state_active_e || instance->state == controller_instance_state_busy_e) { + f_thread_unlock(&instance->lock); + f_thread_unlock(&instance->active); - f_thread_mutex_lock(&instance->wait_lock); - f_thread_condition_signal_all(&instance->wait); - f_thread_mutex_unlock(&instance->wait_lock); - } + return F_busy; + } + + // The thread is done, so close the thread. + if (instance->state == controller_instance_state_done_e) { + controller_thread_join(&instance->id_thread); - instance->id = at; + f_thread_mutex_lock(&instance->wait_lock); + f_thread_condition_signal_all(&instance->wait); + f_thread_mutex_unlock(&instance->wait_lock); } + instance->id = at; + f_thread_unlock(&instance->lock); status_lock = controller_lock_write_instance(instance, &instance->lock); @@ -762,11 +755,7 @@ extern "C" { f_thread_unlock(&instance->active); - if (F_status_is_error(status)) { - return status; - } - - return F_okay; + return F_status_is_error(status) ? status : F_okay; } #endif // _di_controller_rule_instance_begin_ diff --git a/sources/c/main/rule/wait.c b/sources/c/main/rule/wait.c index 9d984e2..177c756 100644 --- a/sources/c/main/rule/wait.c +++ b/sources/c/main/rule/wait.c @@ -24,9 +24,8 @@ extern "C" { } f_status_t status = F_okay; - - bool required_not_run = F_false; - bool skip = F_false; + uint8_t required_not_run = F_false; + uint8_t skip = F_false; f_number_unsigned_t i = 0; f_number_unsigned_t j = 0; diff --git a/sources/c/main/thread/cleanup.c b/sources/c/main/thread/cleanup.c index a14465f..03fa907 100644 --- a/sources/c/main/thread/cleanup.c +++ b/sources/c/main/thread/cleanup.c @@ -39,6 +39,10 @@ extern "C" { if (!main->thread.instances.array[i]) continue; + if (i >= main->thread.instances.used) { + status = controller_instance_initialize(&main->thread.instances.array[i]); + } + instance = main->thread.instances.array[i]; // If "active" has a read lock, then do not attempt to clean it. diff --git a/sources/c/main/thread/entry.c b/sources/c/main/thread/entry.c index 168ad7f..f02bdeb 100644 --- a/sources/c/main/thread/entry.c +++ b/sources/c/main/thread/entry.c @@ -9,7 +9,7 @@ extern "C" { if (!argument) return 0; - f_thread_cancel_state_set(PTHREAD_CANCEL_DEFERRED, 0); + f_thread_cancel_state_set(PTHREAD_CANCEL_DEFERRED, 0); // TODO: need to setup thread states as deferred before starting the thread to avoid needing to do this call. controller_t * const main = (controller_t *) argument;