make UNLANG_TYPE_MODULE skip using resumable frames

The basic tests seem to work.  So let's hope that the rest works.

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index e09806d1007fe6c0e9e8d563f548af4f58c9ea48..8389298f29f8970ee1e0e591ebbfa3d4aca01e3e 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -77,6 +77,7 @@ static void frame_dump(REQUEST *request, unlang_stack_frame_t *frame)
        RDEBUG2("repeat         %s", is_repeatable(frame) ? "yes" : "no");
        RDEBUG2("break_point    %s", is_break_point(frame) ? "yes" : "no");
        RDEBUG2("return_point   %s", is_return_point(frame) ? "yes" : "no");
+       RDEBUG2("resumable      %s", is_resumable(frame) ? "yes" : "no");
        REXDENT();
 }
 
@@ -438,6 +439,7 @@ static inline void frame_cleanup(unlang_stack_frame_t *frame)
        repeatable_clear(frame);
        break_point_clear(frame);
        return_point_clear(frame);
+       resumable_clear(frame);
        if (frame->state) TALLOC_FREE(frame->state);
 }
 
@@ -610,8 +612,10 @@ static inline unlang_frame_action_t frame_eval(REQUEST *request, unlang_stack_fr
 
                                return UNLANG_FRAME_ACTION_YIELD;
 
+                       case UNLANG_TYPE_MODULE: /* until we get rid of UNLANG_TYPE_RESUME */
                        case UNLANG_TYPE_RESUME:
                                repeatable_set(frame);
+                               resumable_set(frame);
                                RDEBUG4("** [%i] %s - yielding with current (%s %d)", stack->depth, __FUNCTION__,
                                        fr_table_str_by_value(mod_rcode_table, frame->result, "<invalid>"),
                                        frame->priority);
@@ -1128,16 +1132,30 @@ static void frame_signal(REQUEST *request, fr_state_signal_t action, int limit)
                /*
                 *      Be gracious in errors.
                 */
-               if (frame->instruction->type != UNLANG_TYPE_RESUME) continue;
+               if (!is_resumable(frame)) continue;
 
-               mr = unlang_generic_to_resume(frame->instruction);
+               switch (frame->instruction->type) {
+               case UNLANG_TYPE_MODULE: /* until we get rid of UNLANG_TYPE_RESUME */
+                       if (!frame->signal) continue;
 
-               /*
-                *      No signal handler for this frame type
-                */
-               if (!unlang_ops[mr->parent->type].signal) continue;
+                       frame->signal(request, NULL, action);
+                       break;
+
+               case UNLANG_TYPE_RESUME:
+                       mr = unlang_generic_to_resume(frame->instruction);
+
+                       /*
+                        *      No signal handler for this frame type
+                        */
+                       if (!unlang_ops[mr->parent->type].signal) continue;
+
+                       unlang_ops[mr->parent->type].signal(request, mr->rctx, action);
+                       break;
 
-               unlang_ops[mr->parent->type].signal(request, mr->rctx, action);
+               default:
+                       rad_assert(0);
+                       break;
+               }
        }
        stack->depth = depth;                           /* Reset */
 }
@@ -1199,26 +1217,39 @@ void unlang_interpret_resumable(REQUEST *request)
        unlang_stack_frame_t            *frame = &stack->frame[stack->depth];
 
        /*
+        *      The request hasn't yield, OR it's already been marked
+        *      as runnable.  Don't do anything.
+        *
         *      The IO code, or children have no idea where they're
-        *      being called from.  They just mark the parent
+        *      being called from.  They just ask to mark the parent
         *      resumable when they're done.  So we have to check here
-        *      for a RESUME frame.  If the parent called the child
-        *      directly, then there's no RESUME frame.  When the
-        *      child is done, the parent will automatically continue
-        *      running.  We threfore don't need to insert the parent
-        *      into the backlog.
+        *      if this request is resumable.
+        *
+        *      If the parent called the child directly, then the
+        *      parent hasn't yielded, so it isn't resumable.  When
+        *      the child is done, the parent will automatically
+        *      continue running.  We therefore don't need to insert
+        *      the parent into the backlog.
+        *
+        *      Multiple child request may also mark a parent request
+        *      runnable, before the parent request starts running.
+        */
+       if (!is_resumable(frame) || (request->runnable_id >= 0)) return;
+
+       /*
         */
-       if (frame->instruction->type != UNLANG_TYPE_RESUME) {
+       switch (frame->instruction->type) {
+       case UNLANG_TYPE_MODULE:
+       case UNLANG_TYPE_RESUME:
+               break;
+
+       default:
                return;
        }
 
        rad_assert(request->backlog != NULL);
 
-       /*
-        *      Multiple child request may mark a request runnable,
-        *      before it is enabled for running.
-        */
-       if (request->runnable_id < 0) fr_heap_insert(request->backlog, request);
+       fr_heap_insert(request->backlog, request);
 }
 
 /** Callback for handling resumption frames

diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c
index bd3592769682a585ac956e5431477211f3b4dda3..d4d3bda3e955d145f71f2e8892bcc0349d292661 100644 (file)
--- a/src/lib/unlang/module.c
+++ b/src/lib/unlang/module.c
@@ -336,7 +336,7 @@ void unlang_module_push(rlm_rcode_t *out, REQUEST *request,
        unlang_frame_state_module_t     *ms;
        unlang_stack_t                  *stack = request->stack;
        unlang_stack_frame_t            *frame;
-       unlang_module_t                 *mi;
+       unlang_module_t                 *sp;
 
        static unlang_t module_instruction = {
                .type = UNLANG_TYPE_MODULE,
@@ -363,17 +363,17 @@ void unlang_module_push(rlm_rcode_t *out, REQUEST *request,
        ms->thread = module_thread(module_instance);
        rad_assert(ms->thread != NULL);
 
-       MEM(mi = talloc_zero(ms, unlang_module_t));     /* Free at the same time as the state */
-       mi->self = module_instruction;
-       mi->self.name = module_instance->name;
-       mi->self.debug_name = mi->self.name;
-       mi->module_instance = module_instance;
-       mi->method = method;
+       MEM(sp = talloc_zero(ms, unlang_module_t));     /* Free at the same time as the state */
+       sp->self = module_instruction;
+       sp->self.name = module_instance->name;
+       sp->self.debug_name = sp->self.name;
+       sp->module_instance = module_instance;
+       sp->method = method;
 
        /*
         *      Push a new module frame onto the stack
         */
-       unlang_interpret_push(request, unlang_module_to_generic(mi), RLM_MODULE_UNKNOWN, UNLANG_NEXT_STOP, top_frame);
+       unlang_interpret_push(request, unlang_module_to_generic(sp), RLM_MODULE_UNKNOWN, UNLANG_NEXT_STOP, top_frame);
        frame = &stack->frame[stack->depth];
        frame->state = ms;
 }
@@ -507,6 +507,17 @@ rlm_rcode_t unlang_module_yield_to_section(REQUEST *request, CONF_SECTION *subcs
        return RLM_MODULE_YIELD;
 }
 
+/*
+ *     @todo - hacks until we get rid of UNLANG_TYPE_RESUME
+ */
+static unlang_action_t unlang_module_resume(REQUEST *request, rlm_rcode_t *presult, UNUSED void *rctx);
+
+static unlang_action_t unlang_module_process_resume(REQUEST *request, rlm_rcode_t *presult)
+{
+       return unlang_module_resume(request, presult, NULL);
+}
+
+
 /** Yield a request back to the interpreter from within a module
  *
  * This passes control of the request back to the unlang interpreter, setting
@@ -531,38 +542,18 @@ rlm_rcode_t unlang_module_yield(REQUEST *request,
 {
        unlang_stack_t                  *stack = request->stack;
        unlang_stack_frame_t            *frame = &stack->frame[stack->depth];
-       unlang_resume_t                 *mr;
-
-       rad_assert(stack->depth > 0);
+       unlang_frame_state_module_t     *ms = talloc_get_type_abort(frame->state,
+                                                                   unlang_frame_state_module_t);
 
        REQUEST_VERIFY(request);        /* Check the yielded request is sane */
 
-       switch (frame->instruction->type) {
-       case UNLANG_TYPE_MODULE:
-               mr = unlang_interpret_resume_alloc(request, (void *)resume, (void *)signal, rctx);
-               if (!fr_cond_assert(mr)) {
-                       return RLM_MODULE_FAIL;
-               }
-               return RLM_MODULE_YIELD;
+       ms->rctx = rctx;
+       ms->resume = resume;
+       ms->signal = signal;
 
-       case UNLANG_TYPE_RESUME:
-               mr = talloc_get_type_abort(frame->instruction, unlang_resume_t);
-               rad_assert(mr->parent->type == UNLANG_TYPE_MODULE);
+       frame->process = unlang_module_process_resume;
 
-               /*
-                *      Re-use the current RESUME frame, but over-ride
-                *      the callbacks and context.
-                */
-               mr->resume = (void *)resume;
-               mr->signal = (void *)signal;
-               mr->rctx = rctx;
-
-               return RLM_MODULE_YIELD;
-
-       default:
-               rad_assert(0);
-               return RLM_MODULE_FAIL;
-       }
+       return RLM_MODULE_YIELD;
 }
 
 /*
@@ -699,30 +690,20 @@ done:
  * @param[in] rctx             createed by #unlang_module.
  * @param[in] action           to signal.
  */
-static void unlang_module_signal(REQUEST *request, void *rctx, fr_state_signal_t action)
+static void unlang_module_signal(REQUEST *request, UNUSED void *rctx, fr_state_signal_t action)
 {
-       unlang_stack_frame_t            *frame;
        unlang_stack_t                  *stack = request->stack;
-       unlang_resume_t                 *mr;
-       unlang_module_t         *mc;
+       unlang_stack_frame_t            *frame = &stack->frame[stack->depth];
+       unlang_frame_state_module_t     *ms = talloc_get_type_abort(frame->state, unlang_frame_state_module_t);
+       unlang_module_t                 *sp = unlang_generic_to_module(frame->instruction);
        char const                      *caller;
 
-       unlang_frame_state_module_t     *ms = NULL;
-
-       rad_assert(stack->depth > 0);
-
-       frame = &stack->frame[stack->depth];
-
-       mr = unlang_generic_to_resume(frame->instruction);
-       if (!mr->signal) return;
-
-       mc = unlang_generic_to_module(mr->parent);
-       ms = talloc_get_type_abort(frame->state, unlang_frame_state_module_t);
+       if (!ms->signal) return;
 
        caller = request->module;
-       request->module = mc->module_instance->name;
-       ((fr_unlang_module_signal_t)mr->signal)(mc->module_instance->dl_inst->data, ms->thread->data, request,
-                                               rctx, action);
+       request->module = sp->module_instance->name;
+       ms->signal(sp->module_instance->dl_inst->data, ms->thread->data, request,
+                  ms->rctx, action);
        request->module = caller;
 }
 
@@ -730,33 +711,28 @@ static unlang_action_t unlang_module_resume(REQUEST *request, rlm_rcode_t *presu
 {
        unlang_stack_t                  *stack = request->stack;
        unlang_stack_frame_t            *frame = &stack->frame[stack->depth];
-       unlang_t                        *instruction = frame->instruction;
-       unlang_resume_t                 *mr = unlang_generic_to_resume(instruction);
-       unlang_module_t                 *mc = unlang_generic_to_module(mr->parent);
-       int                             stack_depth = stack->depth;
-       char const                      *caller;
+       unlang_frame_state_module_t     *ms = talloc_get_type_abort(frame->state, unlang_frame_state_module_t);
+       unlang_module_t                 *sp = unlang_generic_to_module(frame->instruction);
+       char const                      *caller;
        rlm_rcode_t                     rcode;
+       int                             stack_depth = stack->depth;
 
-       unlang_frame_state_module_t     *ms = NULL;
-
-       rad_assert(mr->parent->type == UNLANG_TYPE_MODULE);
-
-       ms = talloc_get_type_abort(frame->state, unlang_frame_state_module_t);
+       rad_assert(ms->resume != NULL);
 
        /*
         *      Lock is noop unless instance->mutex is set.
         */
        caller = request->module;
-       request->module = mc->module_instance->name;
-       safe_lock(mc->module_instance);
-       rcode = request->rcode = ((fr_unlang_module_resume_t)mr->resume)(mc->module_instance->dl_inst->data,
-                                                                        ms->thread->data, request, mr->rctx);
-       safe_unlock(mc->module_instance);
+       request->module = sp->module_instance->name;
+       safe_lock(sp->module_instance);
+       rcode = request->rcode = ms->resume(sp->module_instance->dl_inst->data,
+                                           ms->thread->data, request, ms->rctx);
+       safe_unlock(sp->module_instance);
        request->module = caller;
 
        if (rcode != RLM_MODULE_YIELD) ms->thread->active_callers--;
 
-       RDEBUG2("%s (%s)", instruction->name ? instruction->name : "",
+       RDEBUG2("%s (%s)", frame->instruction->name ? frame->instruction->name : "",
                fr_table_str_by_value(mod_rcode_table, rcode, "<invalid>"));
 
        switch (rcode) {

diff --git a/src/lib/unlang/module_priv.h b/src/lib/unlang/module_priv.h
index 3c026ecf3ea8a135665f2f847d46d3c213f42409..47c3c41486604c6b413e8f67b01fc5521e52636e 100644 (file)
--- a/src/lib/unlang/module_priv.h
+++ b/src/lib/unlang/module_priv.h
@@ -45,6 +45,9 @@ typedef struct {
 typedef struct {
        rlm_rcode_t                     *presult;               //!< Where to store the result.
        module_thread_instance_t        *thread;                //!< thread-local data for this module
+       void                            *rctx;                  //!< for resume / signal
+       fr_unlang_module_resume_t       resume;                 //!< resumption handler
+       fr_unlang_module_signal_t       signal;                 //!< for signal handlers
 } unlang_frame_state_module_t;
 
 static inline unlang_module_t *unlang_generic_to_module(unlang_t *p)

diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h
index 83b58f87995832461efdd449e1cfec834b1e8fb0..e7e28f66d4b6315e4587fcb173c30589c8c447d0 100644 (file)
--- a/src/lib/unlang/unlang_priv.h
+++ b/src/lib/unlang/unlang_priv.h
@@ -293,21 +293,25 @@ typedef struct {
 #define UNWIND_FLAG_BREAK_POINT                0x04                    //!< 'break' stops here.
 #define UNWIND_FLAG_RETURN_POINT       0x08                    //!< 'return' stops here.
 #define UNWIND_FLAG_NO_CLEAR           0x10                    //!< Keep unwinding, don't clear the unwind flag.
+#define UNWIND_FLAG_RESUMABLE          0x20                    //!< Resume point, temporary until we get rid of UNLANG_TYPE_RESUME
 
 static inline void repeatable_set(unlang_stack_frame_t *frame)         { frame->uflags |= UNWIND_FLAG_REPEAT; }
 static inline void top_frame_set(unlang_stack_frame_t *frame)          { frame->uflags |= UNWIND_FLAG_TOP_FRAME; }
 static inline void break_point_set(unlang_stack_frame_t *frame)                { frame->uflags |= UNWIND_FLAG_BREAK_POINT; }
 static inline void return_point_set(unlang_stack_frame_t *frame)       { frame->uflags |= UNWIND_FLAG_RETURN_POINT; }
+static inline void resumable_set(unlang_stack_frame_t *frame)          { frame->uflags |= UNWIND_FLAG_RESUMABLE; }
 
 static inline void repeatable_clear(unlang_stack_frame_t *frame)       { frame->uflags &= ~UNWIND_FLAG_REPEAT; }
 static inline void top_frame_clear(unlang_stack_frame_t *frame)                { frame->uflags &= ~UNWIND_FLAG_TOP_FRAME; }
 static inline void break_point_clear(unlang_stack_frame_t *frame)      { frame->uflags &= ~UNWIND_FLAG_BREAK_POINT; }
 static inline void return_point_clear(unlang_stack_frame_t *frame)     { frame->uflags &= ~UNWIND_FLAG_RETURN_POINT; }
+static inline void resumable_clear(unlang_stack_frame_t *frame)        { frame->uflags &= ~UNWIND_FLAG_RESUMABLE; }
 
 static inline bool is_repeatable(unlang_stack_frame_t *frame)          { return frame->uflags & UNWIND_FLAG_REPEAT; }
 static inline bool is_top_frame(unlang_stack_frame_t *frame)           { return frame->uflags & UNWIND_FLAG_TOP_FRAME; }
 static inline bool is_break_point(unlang_stack_frame_t *frame)         { return frame->uflags & UNWIND_FLAG_BREAK_POINT; }
 static inline bool is_return_point(unlang_stack_frame_t *frame)        { return frame->uflags & UNWIND_FLAG_RETURN_POINT; }
+static inline bool is_resumable(unlang_stack_frame_t *frame)           { return frame->uflags & UNWIND_FLAG_RESUMABLE; }
 
 static inline unlang_action_t unwind_to_break(unlang_stack_t *stack)
 {