From 3f97e9398074dc2c3b47118ba540ad9e9ccda7f2 Mon Sep 17 00:00:00 2001
From: Kevin Day <Kevin@kevux.org>
Date: Sat, 6 Jul 2024 18:58:47 -0500
Subject: [PATCH] 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.
---
 sources/c/main/common/type/instance.c | 69 ++++++++++++++++++++++--
 sources/c/main/common/type/instance.h | 35 +++++++++++--
 sources/c/main/common/type/thread.c   |  2 +-
 sources/c/main/instance/prepare.c     | 29 ++++++++--
 sources/c/main/instance/prepare.h     |  2 +-
 sources/c/main/lock.c                 |  2 +
 sources/c/main/process.c              | 12 ++---
 sources/c/main/rule/instance.c        | 99 ++++++++++++++++-------------------
 sources/c/main/rule/wait.c            |  5 +-
 sources/c/main/thread/cleanup.c       |  4 ++
 sources/c/main/thread/entry.c         |  2 +-
 11 files changed, 180 insertions(+), 81 deletions(-)

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;
 
-- 
1.8.3.1