correct behavior of parallel

it helps to actually set p_result when pushing the child.

Then, update the tests to match the documented behavior.

diff --git a/src/lib/unlang/parallel.c b/src/lib/unlang/parallel.c
index 992f402c65cb004273aa7dcae2fe44117c695718..4eb0e21fc58f8ca9a21f53d938aec27eb4256176 100644 (file)
--- a/src/lib/unlang/parallel.c
+++ b/src/lib/unlang/parallel.c
@@ -169,14 +169,14 @@ static unlang_action_t unlang_parallel_resume(unlang_result_t *p_result, request
                /*
                 *      Do priority over-ride.
                 */
-               if (cr->result.priority > p_result->priority) {
+               if (cr->result.priority > state->result.priority) {
                        RDEBUG4("** [%i] %s - overwriting existing result (%s %d) from higher priority to (%s %d)",
                                stack_depth_current(request), __FUNCTION__,
-                               fr_table_str_by_value(mod_rcode_table, p_result->rcode, "<invalid>"),
-                               p_result->priority,
+                               fr_table_str_by_value(mod_rcode_table, state->result.rcode, "<invalid>"),
+                               state->result.priority,
                                fr_table_str_by_value(mod_rcode_table, cr->result.rcode, "<invalid>"),
                                cr->result.priority);
-                       *p_result = cr->result;
+                       state->result = cr->result;
                }
        }
 
@@ -193,6 +193,7 @@ static unlang_action_t unlang_parallel_resume(unlang_result_t *p_result, request
                state->children[i].state = CHILD_FREED;
        }
 
+       *p_result = state->result;
        return UNLANG_ACTION_CALCULATE_RESULT;
 }
 
@@ -224,8 +225,7 @@ static unlang_action_t unlang_parallel(unlang_result_t *p_result, request_t *req
        }
 
        (void) talloc_set_type(state, unlang_parallel_state_t);
-       state->result = RLM_MODULE_NOOP;
-       state->priority = MOD_ACTION_NOT_SET;                           /* as-yet unset */
+       state->result = UNLANG_RESULT_NOT_SET;
        state->detach = gext->detach;
        state->clone = gext->clone;
        state->num_children = g->num_children;

diff --git a/src/lib/unlang/parallel_priv.h b/src/lib/unlang/parallel_priv.h
index a980f5b2efed4fc9f0dc987f853568dd3874abfb..eb71d1cd6ad81a4adcb5d8c3cf4e8c2bfb0617c9 100644 (file)
--- a/src/lib/unlang/parallel_priv.h
+++ b/src/lib/unlang/parallel_priv.h
@@ -34,8 +34,7 @@ extern "C" {
 #endif
 
 typedef struct {
-       rlm_rcode_t                     result;
-       int                             priority;
+       unlang_result_t                 result;         //!< our result
 
        unsigned int                    num_children;   //!< How many children are executing.
        unsigned int                    num_runnable;   //!< How many children are complete.

diff --git a/src/tests/keywords/parallel-child-timeout b/src/tests/keywords/parallel-child-timeout
index 818d9d527b593708c8b5d646eb22caf8ba8200eb..3d7cc2f50be1c80c89d6841c5c341ecc9e0b7c0b 100644 (file)
--- a/src/tests/keywords/parallel-child-timeout
+++ b/src/tests/keywords/parallel-child-timeout
@@ -1,23 +1,30 @@
 #
 #  PRE: parallel
 #
-try {
-       parallel {
-               timeout 1s {
-                       %time.advance(2s)
-                       parent.control.NAS-Port += 1
-               }
-               # Ensure a timeout in one child does not affect the other
-               group {
-                       parent.control.NAS-Port += 1
-                       ok
-               }
+parallel {
+       timeout 1s {
+               %time.advance(2s)
+               parent.control.Class := 0x01
        }
-}
-catch timeout {
-       if (!(%{control.NAS-Port[#]} == 1)) {
-               test_fail
+       # Ensure a timeout in one child does not affect the other
+       group {
+               parent.control.Class := 0x02
+               ok
        }
+}
 
-       success
+#
+#  This assignment should not have been run
+#
+if (control.Class == 0x01) {
+       test_fail
 }
+
+#
+#  But this assignment should have run.
+#
+if (control.Class != 0x02) {
+       test_fail
+}
+
+success

diff --git a/src/tests/keywords/parallel-parent-and-child-timeout b/src/tests/keywords/parallel-parent-and-child-timeout
deleted file mode 100644 (file)
index cf668e7..0000000
--- a/src/tests/keywords/parallel-parent-and-child-timeout
+++ /dev/null
@@ -1,41 +0,0 @@
-#
-#  PRE: parallel
-#
-try {
-       parallel {
-               # This child should get cancelled by the timeout section
-               timeout 1s {
-                       parallel {
-                               group {
-                                       delay_10s
-                                       outer.control += {
-                                               NAS-Port = 1
-                                       }
-                                       ok
-                               }
-                               group {
-                                       outer.control += {
-                                               NAS-Port = 1
-                                       }
-                                       # Trigger the parent's timeout section so we don't have to wait
-                                       %time.advance(2s)
-                                       ok
-                               }
-                       }
-               }
-               # This child should complete
-               group {
-                       outer.control += {
-                               NAS-Port = 1
-                       }
-                       ok
-               }
-       }
-}
-catch timeout {
-       if (!(%{control.NAS-Port[#]} == 2)) {
-               test_fail
-       }
-
-       success
-}

diff --git a/src/tests/keywords/parallel-rcode b/src/tests/keywords/parallel-rcode
index e44f572325241b633905993e7d13ed9ffc3761f9..eae154eb4e208d61e5d0a7f42e528b4f3d00dcdd 100644 (file)
--- a/src/tests/keywords/parallel-rcode
+++ b/src/tests/keywords/parallel-rcode
@@ -2,66 +2,47 @@
 #  PRE: parallel
 #
 
-#
-#  Ensure that a failing item causes the parallel section to return by default [1/2]
-#
-group {
-       parallel {
-               fail    # Runs
-               ok      # Should not run (would override fail - same priority)
-               ok
-       }
-
-       actions {
-               fail = 1
-       }
-}
-if (!fail) {
-       test_fail
+noop {
+     noop = 1
 }
 
 #
-#  Ensure that a failing item causes the parallel section to return by default [2/2]
+#  Ensure that a failing item causes the parallel section to return by default [1/2]
 #
 group {
        parallel {
-               fail
-               group { # This should *NOT* be dispatched
-                       Filter-Id := 'foo'
+               fail {
+                       fail = 2
+               }
+               updated {
+                       updated = 3
+               }
+               ok {
+                       ok = 4
                }
-       }
-       actions {
-               fail = 1
        }
 }
-if (!fail) {
-       test_fail
-}
-if (Filter-Id) {
+if (!ok) {
        test_fail
 }
 
 #
-#  If the rcode for the failing item is no longer action return, all items should be
-#  executed.
+#  Ensure that a failing item does NOT causes the parallel section to
+#  return.  Instead, we run all children to the end.  The result is
+#  the same as if all children had run sequentially.
 #
 group {
        parallel {
-               fail {
-                       fail = 10       # Higher priority than ok and no longer action return
-               }
-               group { # This should now be dispatched
-                       parent.request.Filter-Id := 'foo'
+               fail
+               group { # This SHOULD be dispatched
+                       parent.control.Class := 0x01
                }
        }
-       actions {
-               fail = 1
-       }
 }
-if (!fail) {
+if (!ok) {
        test_fail
 }
-if (!(Filter-Id == 'foo')) {
+if (control.Class != 0x01) {
        test_fail
 }