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 c2d73a97080a6171468be664dccf2a9d26a9a376..5c37b047437da8a049fd010a9a48d77836cf40ff 100644 (file)
--- 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 0664f41eb96567325292e901c969095ea51f0e4d..12ffec776b138e79960207291ec9e365f4e0bdb6 100644 (file)
--- 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 16e729cd2c855c7a117d63afb75c35b05c18b4dc..9d48867e367f594752c982bde64bb5c311942dd6 100644 (file)
--- 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 41974971891bd5038b3cc4cb1044981890cef5dd..b59345727a47afd4e83ba0c4fe38fc86f594ea7c 100644 (file)
--- 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 3f836a76fabedce1d45ad35b5cd8ce9cf210f16c..c920acba48b055465d5f60c04f83780d6c220251 100644 (file)
--- 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