From: Kevin Day <thekevinday@gmail.com>
Date: Sun, 24 Oct 2021 03:42:34 +0000 (-0500)
Subject: Bugfix: Require is not properly triggering failure on error.
X-Git-Tag: 0.5.6~1
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=f2eb62222636b24cd9ad50025b5e18011ff06cc3;p=fll

Bugfix: Require is not properly triggering failure on error.

Get and handle the status code of controller_rule_wait_all().

Fix cases where "status" is being used where "status_lock" is better suited.

Remove unnecessary double assignment of status error.
In this case, only when F_lock is returned should the status be set.
This may need to be revisited and have the called function also set error on F_lock rather than handling here.

For asynchronous functions controller_rule_wait_all() is not properly handling the error.
This is happening because "required" is being incorrectly used.
For this function "required" is design to only wait for all required processes only and not limit the required validation.
Removing this fixing the problem so that when an asynchronous process that is required fails, the failure is propagated to the waiting process.

Add "require" in the htop example as a place for testing.

Add additional comments for clarifications and cleanup existing comments.

Remove unnecessary ready wait assignment.
---

diff --git a/level_3/controller/c/private-controller.c b/level_3/controller/c/private-controller.c
index c2d73a9..5c37b04 100644
--- a/level_3/controller/c/private-controller.c
+++ b/level_3/controller/c/private-controller.c
@@ -557,8 +557,9 @@ extern "C" {
               controller_print_unlock_flush(global.main->warning.to, global.thread);
             }
           }
-
-          global.setting->ready = controller_setting_ready_wait;
+          else {
+            global.setting->ready = controller_setting_ready_wait;
+          }
         }
         else if (actions->array[cache->ats.array[at_j]].type == controller_entry_action_type_item) {
           error_has = F_false;
@@ -883,7 +884,8 @@ extern "C" {
             }
 
             if (global->main->parameters[controller_parameter_validate].result == f_console_result_none) {
-              controller_rule_wait_all(is_entry, *global, F_false, process);
+              status = controller_rule_wait_all(is_entry, *global, F_false, process);
+              if (F_status_is_error(status)) return status;
             }
           }
 
diff --git a/level_3/controller/c/private-rule.c b/level_3/controller/c/private-rule.c
index 0664f41..12ffec7 100644
--- a/level_3/controller/c/private-rule.c
+++ b/level_3/controller/c/private-rule.c
@@ -2902,7 +2902,6 @@ extern "C" {
     }
 
     if ((process->options & controller_process_option_wait_d) && F_status_is_error_not(status) && (process->options & controller_process_option_validate_d)) {
-
       status_lock = controller_rule_wait_all_process_type(process->type, global, F_false, process);
 
       if (status_lock == F_signal) {
@@ -3021,6 +3020,7 @@ extern "C" {
       return F_status_set_error(F_lock);
     }
 
+    // Update the global rule status, which is stored separately from the rule status for this process.
     if (controller_rule_find(process->rule.alias, global.setting->rules, &id_rule) == F_true) {
       controller_rule_t *rule = &global.setting->rules.array[id_rule];
 
@@ -3066,6 +3066,7 @@ extern "C" {
     }
 
     f_status_t status = F_none;
+    f_status_t status_lock = F_none;
 
     controller_process_t *process = 0;
 
@@ -3110,15 +3111,15 @@ extern "C" {
         return status;
       }
 
-      status = controller_lock_write_process(process, global.thread, &process->lock);
+      status_lock = controller_lock_write_process(process, global.thread, &process->lock);
 
-      if (status == F_signal || F_status_is_error(status)) {
-        controller_lock_error_critical_print(global.main->error, F_status_set_fine(status), F_false, global.thread);
+      if (status_lock == F_signal || F_status_is_error(status_lock)) {
+        controller_lock_error_critical_print(global.main->error, F_status_set_fine(status_lock), F_false, global.thread);
 
         f_thread_unlock(&process->active);
         f_thread_unlock(&global.thread->lock.process);
 
-        return status;
+        return status_lock;
       }
 
       // once a write lock on the process is achieved, it is safe to unlock the global process read lock.
@@ -3146,14 +3147,14 @@ extern "C" {
 
     f_thread_unlock(&process->lock);
 
-    status = controller_lock_write_process(process, global.thread, &process->lock);
+    status_lock = controller_lock_write_process(process, global.thread, &process->lock);
 
-    if (status == F_signal || F_status_is_error(status)) {
-      controller_lock_error_critical_print(global.main->error, F_status_set_fine(status), F_false, global.thread);
+    if (status_lock == F_signal || F_status_is_error(status_lock)) {
+      controller_lock_error_critical_print(global.main->error, F_status_set_fine(status_lock), F_false, global.thread);
 
       f_thread_unlock(&process->active);
 
-      return status;
+      return status_lock;
     }
 
     process->state = controller_process_state_active;
@@ -3248,15 +3249,16 @@ extern "C" {
     }
 
     if (!action || F_status_is_error(status) && (process->state == controller_process_state_active || process->state == controller_process_state_busy)) {
+      {
+        status_lock = controller_lock_write_process(process, global.thread, &process->lock);
 
-      status = controller_lock_write_process(process, global.thread, &process->lock);
-
-      if (status == F_signal || F_status_is_error(status)) {
-        controller_lock_error_critical_print(global.main->error, F_status_set_fine(status), F_false, global.thread);
+        if (status_lock == F_signal || F_status_is_error(status_lock)) {
+          controller_lock_error_critical_print(global.main->error, F_status_set_fine(status_lock), F_false, global.thread);
 
-        f_thread_unlock(&process->active);
+          f_thread_unlock(&process->active);
 
-        return status;
+          return status_lock;
+        }
       }
 
       if (!action || (options_force & controller_process_option_asynchronous_d)) {
@@ -3377,7 +3379,7 @@ extern "C" {
       }
       else if (!process->action) {
 
-        // this is a "consider" Action, so do not actually execute the rule.
+        // This is a "consider" Action, so do not actually execute the rule.
         f_thread_unlock(&process->lock);
 
         if (options_force & controller_process_option_asynchronous_d) {
@@ -3402,7 +3404,7 @@ extern "C" {
               controller_print_unlock_flush(global.main->error.to, global.thread);
             }
 
-            // never continue on circular recursion errors even in simulate mode.
+            // Never continue on circular recursion errors even in simulate mode.
             status = F_status_set_error(F_recurse);
 
             break;
@@ -3504,11 +3506,8 @@ extern "C" {
       return status_lock;
     }
 
-    if (controller_rule_find(process->rule.alias, global.setting->rules, &id_rule) == F_true) {
-      if (F_status_set_fine(status) == F_lock) {
-        global.setting->rules.array[id_rule].status[process->action] = F_status_set_error(F_failure);
-      }
-      else {
+    if (F_status_set_fine(status) == F_lock) {
+      if (controller_rule_find(process->rule.alias, global.setting->rules, &id_rule) == F_true) {
         global.setting->rules.array[id_rule].status[process->action] = status;
       }
     }
@@ -6446,7 +6445,7 @@ extern "C" {
           if (status_lock == F_signal || F_status_is_error(status_lock)) break;
         }
 
-        if (required && (process_list[i]->options & controller_process_option_require_d)) {
+        if (process_list[i]->options & controller_process_option_require_d) {
           if (controller_rule_status_is_error(process_list[i]->action, process_list[i]->rule)) {
             status = F_status_set_error(F_require);
 
@@ -6479,34 +6478,32 @@ extern "C" {
           break;
         }
 
-        if (required) {
-          if (caller) {
-            status_lock = controller_lock_read_process(caller, global.thread, &process_list[i]->lock);
-          }
-          else {
-            status_lock = controller_lock_read(is_normal, global.thread, &process_list[i]->lock);
-          }
+        if (caller) {
+          status_lock = controller_lock_read_process(caller, global.thread, &process_list[i]->lock);
+        }
+        else {
+          status_lock = controller_lock_read(is_normal, global.thread, &process_list[i]->lock);
+        }
 
-          if (status_lock == F_signal || F_status_is_error(status_lock)) {
-            f_thread_unlock(&process_list[i]->active);
+        if (status_lock == F_signal || F_status_is_error(status_lock)) {
+          f_thread_unlock(&process_list[i]->active);
 
-            break;
-          }
+          break;
+        }
 
-          if ((process_list[i]->options & controller_process_option_require_d)) {
-            f_thread_unlock(&process_list[i]->lock);
+        if ((process_list[i]->options & controller_process_option_require_d)) {
+          f_thread_unlock(&process_list[i]->lock);
 
-            if (controller_rule_status_is_error(process_list[i]->action, process_list[i]->rule)) {
-              status = F_status_set_error(F_require);
+          if (controller_rule_status_is_error(process_list[i]->action, process_list[i]->rule)) {
+            status = F_status_set_error(F_require);
 
-              f_thread_unlock(&process_list[i]->active);
-              break;
-            }
-          }
-          else {
-            f_thread_unlock(&process_list[i]->lock);
+            f_thread_unlock(&process_list[i]->active);
+            break;
           }
         }
+        else {
+          f_thread_unlock(&process_list[i]->lock);
+        }
       }
       else {
         f_thread_unlock(&process_list[i]->lock);
diff --git a/level_3/controller/c/private-rule.h b/level_3/controller/c/private-rule.h
index 16e729c..9d48867 100644
--- a/level_3/controller/c/private-rule.h
+++ b/level_3/controller/c/private-rule.h
@@ -990,7 +990,7 @@ extern "C" {
  *   The global data.
  * @param required
  *   If TRUE, then only process required rules and if a required rule has failed, return.
- *   If FALSE, process all waits, returning normally.
+ *   If FALSE, process all waits, returning normally (required rules still result in failure).
  * @param caller
  *   The process representing the caller so that the process never waits on itself.
  *   (optional) set to 0 when calling from a thread that is not running/executing any process.
@@ -1005,7 +1005,7 @@ extern "C" {
  *    F_require (with error bit set) if a required process is in failed status when required is TRUE.
  */
 #ifndef _di_controller_rule_wait_all_
-  extern f_status_t controller_rule_wait_all(const bool is_normal, const controller_global_t global, const bool required, controller_process_t *process) F_attribute_visibility_internal_d;
+  extern f_status_t controller_rule_wait_all(const bool is_normal, const controller_global_t global, const bool required, controller_process_t *caller) F_attribute_visibility_internal_d;
 #endif // _di_controller_rule_wait_all_
 
 /**
diff --git a/level_3/controller/c/private-thread.c b/level_3/controller/c/private-thread.c
index 4197497..b593457 100644
--- a/level_3/controller/c/private-thread.c
+++ b/level_3/controller/c/private-thread.c
@@ -305,7 +305,7 @@ extern "C" {
         controller_thread_join(&thread.id_signal);
       }
       else if (setting->mode == controller_setting_mode_program) {
-        controller_rule_wait_all(F_true, global, F_false, 0);
+        status = controller_rule_wait_all(F_true, global, F_false, 0);
       }
     }
 
diff --git a/level_3/controller/data/settings/example/entries/htop.entry b/level_3/controller/data/settings/example/entries/htop.entry
index 3f836a7..c920acb 100644
--- a/level_3/controller/data/settings/example/entries/htop.entry
+++ b/level_3/controller/data/settings/example/entries/htop.entry
@@ -6,9 +6,9 @@ setting:
 main:
   failsafe start_top
 
-  start serial s_1 asynchronous
-  start serial s_2 asynchronous
-  start serial s_3 asynchronous
+  start serial s_1 asynchronous require
+  start serial s_2 asynchronous require
+  start serial s_3 asynchronous require
 
   ready wait