Remove interpreter stop callback

Simplify use of request master state

diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c
index 7d53b68d3b3d0616b162fb91e66fb72d4c04cc99..60a59604c956a44c5757cacf7cf9c23698f181ae 100644 (file)
--- a/src/lib/io/worker.c
+++ b/src/lib/io/worker.c
@@ -1166,13 +1166,12 @@ static void _worker_request_done_external(request_t *request, UNUSED rlm_rcode_t
         *
         *      This should never happen otherwise.
         */
-       if (unlikely((request->master_state == REQUEST_STOP_PROCESSING) &&
-                    !fr_channel_active(request->async->channel))) {
+       if (unlikely(!fr_channel_active(request->async->channel))) {
                request_slab_release(request);
                return;
        }
 
-       worker_send_reply(worker, request, request->master_state != REQUEST_STOP_PROCESSING, now);
+       worker_send_reply(worker, request, !unlang_request_is_cancelled(request), now);
        request_slab_release(request);
 }
 
@@ -1255,35 +1254,6 @@ static void _worker_request_detach(request_t *request, void *uctx)
        return;
 }
 
-/** This is called by the interpreter when it wants to stop a request
- *
- * The idea is to get the request into the same state it would be in
- * if the interpreter had just finished with it.
- */
-static void _worker_request_stop(request_t *request, void *uctx)
-{
-       fr_worker_t     *worker = talloc_get_type_abort(uctx, fr_worker_t);
-
-       RDEBUG3("Cleaning up request execution state");
-
-       /*
-        *      Make sure time tracking is always in a
-        *      consistent state when we mark the request
-        *      as done.
-        */
-       if (request->async->tracking.state == FR_TIME_TRACKING_YIELDED) {
-               RDEBUG3("Forcing time tracking to running state, from yielded, for request stop");
-               fr_time_tracking_resume(&request->async->tracking, fr_time());
-       }
-
-       /*
-        *      If the request is in the runnable queue
-        *      yank it back out, so it's not "runnable"
-        *      when we call request done.
-        */
-       if (fr_heap_entry_inserted(request->runnable)) fr_heap_extract(&worker->runnable, request);
-}
-
 /** Request is now runnable
  *
  */
@@ -1497,7 +1467,6 @@ nomem:
                                                        .done_detached = _worker_request_done_detached,
 
                                                        .detach = _worker_request_detach,
-                                                       .stop = _worker_request_stop,
                                                        .yield = _worker_request_yield,
                                                        .resume = _worker_request_resume,
                                                        .mark_runnable = _worker_request_runnable,

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 6d29446e26aff141434e8bededbc69b0cefce1d4..040ab3f3928615ea7a12e585342fb5c5ec1f45b6 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -231,8 +231,8 @@ static int _local_variables_free(unlang_variable_ref_t *ref)
  *     - UNLANG_ACTION_EXECUTE_NEXT do nothing, but just go to the next sibling instruction
  *     - UNLANG_ACTION_STOP_PROCESSING, fatal error, usually stack overflow.
  */
-unlang_action_t unlang_interpret_push_children(rlm_rcode_t *p_result, request_t *request,
-                                             rlm_rcode_t default_rcode, bool do_next_sibling)
+unlang_action_t unlang_interpret_push_children(UNUSED rlm_rcode_t *p_result, request_t *request,
+                                              rlm_rcode_t default_rcode, bool do_next_sibling)
 {
        unlang_stack_t          *stack = request->stack;
        unlang_stack_frame_t    *frame = &stack->frame[stack->depth];   /* Quiet static analysis */
@@ -522,37 +522,17 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
                /*
                 *      Failure testing!
                 */
-               if (request->ins_max && (request->master_state != REQUEST_STOP_PROCESSING)) {
+               if (request->ins_max) {
                        request->ins_count++;
 
                        if (request->ins_count >= request->ins_max) {
                                RERROR("Failing request due to maximum instruction count %" PRIu64, request->ins_max);
 
                                unlang_interpret_signal(request, FR_SIGNAL_CANCEL);
-                               request->master_state = REQUEST_STOP_PROCESSING;
                        }
                }
 #endif
 
-               /*
-                *      unlang_interpret_signal() takes care of
-                *      marking the requests as STOP on a CANCEL
-                *      signal.
-                */
-               if (request->master_state == REQUEST_STOP_PROCESSING) {
-               do_stop:
-                       frame->result = RLM_MODULE_FAIL;
-                       frame->priority = MOD_PRIORITY_MAX;
-
-                       RDEBUG4("** [%i] %s - STOP current subsection with (%s %d)",
-                               stack->depth, __FUNCTION__,
-                               fr_table_str_by_value(mod_rcode_table, frame->result, "<invalid>"),
-                               frame->priority);
-
-                       unwind_all(stack);
-                       return UNLANG_FRAME_ACTION_POP;
-               }
-
                if (!is_repeatable(frame) && has_debug_braces(frame)) {
                        RDEBUG2("%s {", instruction->debug_name);
                        RINDENT();
@@ -605,12 +585,9 @@ unlang_frame_action_t frame_eval(request_t *request, unlang_stack_frame_t *frame
                if (is_cancelled(frame)) goto calculate_result;
 
                switch (ua) {
-               /*
-                *      The request is now defunct, and we should not
-                *      continue processing it.
-                */
                case UNLANG_ACTION_STOP_PROCESSING:
-                       goto do_stop;
+                       unlang_interpret_signal(request, FR_SIGNAL_CANCEL);
+                       goto calculate_result;
 
                /*
                 *      The operation resulted in additional frames
@@ -755,8 +732,6 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
        if (!running) intp->funcs.resume(request, intp->uctx);
 
        for (;;) {
-               fr_assert(request->master_state != REQUEST_STOP_PROCESSING);
-
                RDEBUG4("** [%i] %s - frame action %s", stack->depth, __FUNCTION__,
                        fr_table_str_by_value(unlang_frame_action_table, fa, "<INVALID>"));
                switch (fa) {
@@ -769,11 +744,6 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
 
                        if (fa != UNLANG_FRAME_ACTION_POP) continue;
 
-                       /*
-                        *      We're supposed to stop processing.  Don't pop anything, just stop.
-                        */
-                       if (request->master_state == REQUEST_STOP_PROCESSING) return RLM_MODULE_FAIL;
-
                        /*
                         *      We were executing a frame, frame_eval()
                         *      indicated we should pop it, but we're now at
@@ -805,6 +775,12 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                         *      Head on back up the stack
                         */
                        frame_pop(request, stack);
+
+                       /*
+                        *      Transition back to the C stack
+                        */
+                       if (is_top_frame(frame)) break; /* stop */
+
                        frame = &stack->frame[stack->depth];
                        DUMP_STACK;
 
@@ -813,7 +789,8 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                         *      or anything else that needs to be checked on the way
                         *      back on up the stack.
                         */
-                       if (is_repeatable(frame)) {
+
+                       if (!is_cancelled(frame) && is_repeatable(frame)) {
                                fa = UNLANG_FRAME_ACTION_NEXT;
                                continue;
                        }
@@ -837,20 +814,6 @@ CC_HINT(hot) rlm_rcode_t unlang_interpret(request_t *request, bool running)
                                }
                        }
 
-                       /*
-                        *      If we're done, merge the last stack->result / priority in.
-                        */
-                       if (is_top_frame(frame)) break; /* stop */
-
-                       /*
-                        *      Carry on popping until we find something that shouldn't
-                        *      be cancelled.
-                        */
-                       if (is_cancelled(frame)) {
-                               fa = UNLANG_FRAME_ACTION_POP;
-                               continue;
-                       }
-
                        fa = result_calculate(request, frame, &stack->result, &stack->priority);
 
                        /*
@@ -1068,20 +1031,6 @@ void unlang_interpret_request_done(request_t *request)
        }
 }
 
-static inline CC_HINT(always_inline)
-void unlang_interpret_request_stop(request_t *request)
-{
-       unlang_stack_t          *stack = request->stack;
-       unlang_interpret_t      *intp;
-
-       if (!fr_cond_assert(stack != NULL)) return;
-
-       intp = stack->intp;
-       intp->funcs.stop(request, intp->uctx);
-       request->log.indent.unlang = 0;                 /* nothing unwinds the indentation stack */
-       request->master_state = REQUEST_STOP_PROCESSING;
-}
-
 static inline CC_HINT(always_inline)
 void unlang_interpret_request_detach(request_t *request)
 {
@@ -1127,7 +1076,7 @@ void unlang_stack_signal(request_t *request, fr_signal_t action, int limit)
 
        (void)talloc_get_type_abort(request, request_t);        /* Check the request hasn't already been freed */
 
-       fr_assert(stack->depth > 0);
+       fr_assert(stack->depth >= 1);
 
        /*
         *      Does not complete the unwinding here, just marks
@@ -1148,7 +1097,7 @@ void unlang_stack_signal(request_t *request, fr_signal_t action, int limit)
         *      stack, as modules can push xlats and function
         *      calls.
         */
-       for (i = depth; i > limit; i--) {
+       for (i = depth; i >= limit; i--) {
                frame = &stack->frame[i];
                if (frame->signal) frame->signal(request, frame, action);
        }
@@ -1160,9 +1109,10 @@ void unlang_stack_signal(request_t *request, fr_signal_t action, int limit)
  * outside of the normal processing of the request.
  *
  * @note This does NOT immediately stop the request, it just deliveres
- *      signals, and in the case of a cancel, marks up frames for unwinding.
- *      The request must be marked as runnable, and executed by the
- *      interpreter to complete the cancellation.
+ *      signals, and in the case of a cancel, marks up frames for unwinding
+ *      and adds it to the runnable queue if it's yielded.
+ *
+ * @note This function should be safe to call anywhere.
  *
  * @param[in] request          The current request.
  * @param[in] action           to signal.
@@ -1189,28 +1139,22 @@ void unlang_interpret_signal(request_t *request, fr_signal_t action)
         *      yet should have a stack depth of zero, so we don't
         *      need to do anything.
         */
-       if (stack && (stack->depth > 0)) unlang_stack_signal(request, action, 0);
+       if (stack && (stack->depth > 0)) unlang_stack_signal(request, action, 1);
 
        switch (action) {
        case FR_SIGNAL_CANCEL:
                /*
-                *      Detach the request from the parent to cleanup
-                *      any cross-request pointers.  This is a noop
-                *      if the request is not detachable.
+                *      Let anything that cares, know that the
+                *      request was forcefully stopped.
                 */
-               if (request_is_detachable(request)) unlang_interpret_request_detach(request);
+               request->master_state = REQUEST_STOP_PROCESSING;
 
                /*
-                *      Get the request into a consistent state,
-                *      removing it from any runnable lists.
+                *      If the request is yielded, mark it as runnable
                 */
-               unlang_interpret_request_stop(request);
-
-               /*
-                *      As the request is detached, we call the done_detached
-                *      callback which should free the request.
-                */
-               unlang_interpret_request_done(request);
+               if (is_yielded(&stack->frame[stack->depth]) && !unlang_request_is_scheduled(request)) {
+                       unlang_interpret_mark_runnable(request);
+               }
                break;
 
        case FR_SIGNAL_DETACH:
@@ -1458,7 +1402,7 @@ static void unlang_cancel_event(UNUSED fr_timer_list_t *tl, UNUSED fr_time_t now
         */
        talloc_free(request_data_get(request, (void *)unlang_cancel_xlat, 0));
 
-       unwind_all(request->stack);
+       unlang_interpret_signal(request, FR_SIGNAL_CANCEL);
 }
 
 /** Allows a request to dynamically alter its own lifetime
@@ -1488,7 +1432,7 @@ static xlat_action_t unlang_cancel_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out,
         *      as it expects to see a resume function set.
         */
        if (!timeout || fr_time_delta_eq(timeout->vb_time_delta, fr_time_delta_from_sec(0))) {
-               unwind_all(request->stack);
+               unlang_interpret_signal(request, FR_SIGNAL_CANCEL);
                return XLAT_ACTION_DONE;
        }
 
@@ -1721,7 +1665,6 @@ unlang_interpret_t *unlang_interpret_init(TALLOC_CTX *ctx,
        fr_assert(funcs->done_external);
 
        fr_assert(funcs->detach);
-       fr_assert(funcs->stop);
        fr_assert(funcs->yield);
        fr_assert(funcs->resume);
        fr_assert(funcs->mark_runnable);

diff --git a/src/lib/unlang/interpret.h b/src/lib/unlang/interpret.h
index 9a0ddc9e21d55b9c4b5f6b5141a2fa372af9e614..8cd68a07b98e57796a399621296e1644fa0a2d39 100644 (file)
--- a/src/lib/unlang/interpret.h
+++ b/src/lib/unlang/interpret.h
@@ -113,7 +113,6 @@ typedef struct {
        unlang_request_done_t           done_detached;  //!< Function called when a detached request completes.
 
        unlang_request_init_t           detach;         //!< Function called when a request is detached.
-       unlang_request_stop_t           stop;           //!< function called when a request is signalled to stop.
        unlang_request_yield_t          yield;          //!< Function called when a request yields.
        unlang_request_resume_t         resume;         //!< Function called when a request is resumed.
        unlang_request_runnable_t       mark_runnable;  //!< Function called when a request needs to be

diff --git a/src/lib/unlang/interpret_synchronous.c b/src/lib/unlang/interpret_synchronous.c
index 48896a6a04f210dc4b506732b9e178dfd4d6305f..80bce035bdb92132ef4d2c14190d9156c0cfd182 100644 (file)
--- a/src/lib/unlang/interpret_synchronous.c
+++ b/src/lib/unlang/interpret_synchronous.c
@@ -105,24 +105,6 @@ static void _request_detach(request_t *request, UNUSED void *uctx)
        if (request_detach(request) < 0) RPEDEBUG("Failed detaching request");
 }
 
-/** Request has been stopped
- *
- * Clean up execution state
- */
-static void _request_stop(request_t *request, void *uctx)
-{
-       unlang_interpret_synchronous_t  *intps = uctx;
-
-       RDEBUG3("Synchronous request stopped");
-
-       /*
-        *  The assumption here is that if the request
-        *  not in the runnable queue, and it's not
-        *  currently running, then it must be yielded.
-        */
-       if (fr_heap_extract(&intps->runnable, request) < 0) intps->yielded--;
-}
-
 /** Request is now runnable
  *
  */
@@ -184,7 +166,6 @@ static unlang_interpret_synchronous_t *unlang_interpret_synchronous_alloc(TALLOC
                                                        .done_detached = _request_done_detached,
 
                                                        .detach = _request_detach,
-                                                       .stop = _request_stop,
                                                        .yield = _request_yield,
                                                        .resume = _request_resume,
                                                        .mark_runnable = _request_runnable,

diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h
index 4a0847ee623d4fefe5b91690be5e7d1dedce0390..d02ac1a30e88bb8190c232c84db6b38721faf265 100644 (file)
--- a/src/lib/unlang/unlang_priv.h
+++ b/src/lib/unlang/unlang_priv.h
@@ -484,17 +484,6 @@ static inline unlang_action_t unwind_to_return(unsigned int *return_depth, unlan
        return UNLANG_ACTION_CALCULATE_RESULT;
 }
 
-/** Mark the entire stack as cancelled
- *
- * This cancels all frames in the stack ignoring the top frames.
- *
- * @param[in] stack    The current stack.
- */
-static inline void unwind_all(unlang_stack_t *stack)
-{
-       unwind_to_depth(stack, 1);
-}
-
 static inline unlang_stack_frame_t *frame_current(request_t *request)
 {
        unlang_stack_t *stack = request->stack;