Fix time tracking assert when the server is blocked processing children

diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c
index 903d40a4e5c20102b67a04737e668f14d1fb2628..5c947964b095a117e1699553109fe95f749d6638 100644 (file)
--- a/src/bin/radiusd.c
+++ b/src/bin/radiusd.c
@@ -1223,7 +1223,7 @@ static void sig_fatal(int sig)
         *      Suppress duplicate signals.
         *
         *      For some reason on macOS we get multiple signals
-        *      for the same event (SIGTERM).
+        *      for the same event (SIGINT).
         *
         *      ...this also fixes the problem of the user hammering
         *      Ctrl-C and causing ungraceful exits as we try and

diff --git a/src/lib/server/request.h b/src/lib/server/request.h
index b11f56481400256d941ed1f0b9c70c5b260357d3..1b2934f4a4f6cc23f49072ece970c597a3495827 100644 (file)
--- a/src/lib/server/request.h
+++ b/src/lib/server/request.h
@@ -58,9 +58,9 @@ extern "C" {
 #endif
 
 typedef enum {
-       REQUEST_ACTIVE = 1,
-       REQUEST_STOP_PROCESSING,
-       REQUEST_COUNTED
+       REQUEST_ACTIVE = 1,             //!< Request is active (running or runnable)
+       REQUEST_STOP_PROCESSING,        //!< Request has been signalled to stop
+       REQUEST_DONE,                   //!< Request has completed
 } request_master_state_t;
 #define REQUEST_MASTER_NUM_STATES (REQUEST_COUNTED + 1)
 
@@ -221,13 +221,14 @@ struct request_s {
        char const              *component;     //!< Section the request is in.
        char const              *module;        //!< Module the request is currently being processed by.
 
-       fr_packet_t     *packet;        //!< Incoming request.
-       fr_packet_t     *reply;         //!< Outgoing response.
+       fr_packet_t             *packet;        //!< Incoming request.
+       fr_packet_t             *reply;         //!< Outgoing response.
 
        fr_client_t             *client;        //!< The client that originally sent us the request.
 
        request_master_state_t  master_state;   //!< Set by the master thread to signal the child that's currently
                                                //!< working with the request, to do something.
+       bool                    counted;        //!< Set if the request has been counted in the stats.
 
        rlm_rcode_t             rcode;          //!< Last rcode returned by a module
 

diff --git a/src/lib/server/stats.c b/src/lib/server/stats.c
index 82c71e75b6322481954196ad80d02b6159bce15e..0349e3853f605f4957aad93f21b9abe57b8a124b 100644 (file)
--- a/src/lib/server/stats.c
+++ b/src/lib/server/stats.c
@@ -47,7 +47,7 @@ fr_stats_t radius_acct_stats = FR_STATS_INIT;
 
 void request_stats_final(request_t *request)
 {
-       if (request->master_state == REQUEST_COUNTED) return;
+       if (request->counted) return;
 
 #if 0
        if (!request->listener) return;
@@ -143,7 +143,7 @@ void request_stats_final(request_t *request)
                break;
        }
 
-       request->master_state = REQUEST_COUNTED;
+       request->counted = true;
 }
 
 void radius_stats_init(int flag)

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 3873376d28a586241b1b9a6b0824f3e7e97f264c..a8ba0eaa51adcc722e68333588e692af2f8dc2d8 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -1081,6 +1081,8 @@ void unlang_interpret_request_done(request_t *request)
                intp->funcs.done_detached(request, stack->result, intp->uctx);  /* Callback will usually free the request */
                break;
        }
+
+       request->master_state = REQUEST_DONE;
 }
 
 static inline CC_HINT(always_inline)
@@ -1312,6 +1314,13 @@ bool unlang_request_is_cancelled(request_t const *request)
        return (request->master_state == REQUEST_STOP_PROCESSING);
 }
 
+/** Return whether a request has been marked done
+ */
+bool unlang_request_is_done(request_t const *request)
+{
+       return (request->master_state == REQUEST_DONE);
+}
+
 /** Check if a request as resumable.
  *
  * @param[in] request          The current request.

diff --git a/src/lib/unlang/interpret.h b/src/lib/unlang/interpret.h
index 8e47bd453380ee2e203dbdb95b857bf243d232aa..12740dbb4569642bbea08daf30584919a72b31ce 100644 (file)
--- a/src/lib/unlang/interpret.h
+++ b/src/lib/unlang/interpret.h
@@ -152,6 +152,8 @@ bool                        unlang_request_is_scheduled(request_t const *request);
 
 bool                   unlang_request_is_cancelled(request_t const *request);
 
+bool                   unlang_request_is_done(request_t const *request);
+
 void                   unlang_interpret_request_done(request_t *request);
 
 void                   unlang_interpret_mark_runnable(request_t *request);

diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c
index bc1a93851ce38ba754e48e0480f4198306411ea1..105a806e07dce1ed415391142632d0ca6d9a1365 100644 (file)
--- a/src/lib/unlang/subrequest.c
+++ b/src/lib/unlang/subrequest.c
@@ -35,8 +35,8 @@ RCSID("$Id$")
 /** Send a signal from parent request to subrequest
  *
  */
-static void unlang_subrequest_parent_signal(UNUSED request_t *request, unlang_stack_frame_t *frame,
-                                           fr_signal_t action)
+static void unlang_subrequest_signal_child(UNUSED request_t *request, unlang_stack_frame_t *frame,
+                                          fr_signal_t action)
 {
        unlang_frame_state_subrequest_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_subrequest_t);
        request_t                       *child = talloc_get_type_abort(state->child, request_t);
@@ -50,6 +50,27 @@ static void unlang_subrequest_parent_signal(UNUSED request_t *request, unlang_st
         */
        fr_assert(action != FR_SIGNAL_DETACH);
 
+       /*
+        *      If the server is stopped, inside a breakpoint,
+        *      whilst processing a child, on resumption both
+        *      requests (parent and child) may need to be
+        *      cancelled as they've both hit max request_time.
+        *
+        *      Sometimes the child will run to completion before
+        *      the cancellation is processed, but the parent
+        *      will still be cancelled.
+        *
+        *      When the parent is cancelled this function is
+        *      executed, which will signal an already stopped
+        *      child to cancel itself.
+        *
+        *      This triggers asserts in the time tracking code.
+        *
+        *      ...so we check to see if the child is done before
+        *      sending a signal.
+        */
+       if (unlang_request_is_done(child)) return;
+
        /*
         *      Forward other signals to the child
         */
@@ -277,7 +298,7 @@ int unlang_subrequest_op_init(void)
                        &(unlang_op_t){
                                .name = "subrequest",
                                .interpret = unlang_subrequest_parent_init,
-                               .signal = unlang_subrequest_parent_signal,
+                               .signal = unlang_subrequest_signal_child,
                                .debug_braces = true,
                                .frame_state_size = sizeof(unlang_frame_state_subrequest_t),
                                .frame_state_type = "unlang_frame_state_subrequest_t",

diff --git a/src/lib/unlang/subrequest_child.c b/src/lib/unlang/subrequest_child.c
index 9e8bd9f9758cdd3f69e2a8802fdd5a33bf9e489a..365c98c1f2e643a81ddd2b5d34dffcedcc8cf0c0 100644 (file)
--- a/src/lib/unlang/subrequest_child.c
+++ b/src/lib/unlang/subrequest_child.c
@@ -143,6 +143,8 @@ static void unlang_subrequest_child_signal(request_t *request, fr_signal_t actio
                FALL_THROUGH;
 
        case FR_SIGNAL_CANCEL:
+               RDEBUG3("Removing subrequest from parent, and marking parent as runnable");
+
                /*
                 *      Indicate to the parent there's no longer a child
                 */