From: Arran Cudbard-Bell Date: Wed, 30 Apr 2025 18:15:26 +0000 (-0400) Subject: Remove timeout { ... } catch { ... } and add support for timeout rcodes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c26152f32d9d9c93a8c6f8f48132bf2309497c90;p=thirdparty%2Ffreeradius-server.git Remove timeout { ... } catch { ... } and add support for timeout rcodes timeout { ... } now sets a timeout rcode that can be caught like any other code. --- diff --git a/src/lib/unlang/catch.c b/src/lib/unlang/catch.c index 78dd113ea1..e207edbac9 100644 --- a/src/lib/unlang/catch.c +++ b/src/lib/unlang/catch.c @@ -49,7 +49,7 @@ static unlang_action_t unlang_catch(rlm_rcode_t *p_result, request_t *request, u #ifndef NDEBUG unlang_catch_t const *c = unlang_generic_to_catch(frame->instruction); - fr_assert(c->timeout || c->catching[*p_result]); + fr_assert(c->catching[*p_result]); #endif /* diff --git a/src/lib/unlang/catch_priv.h b/src/lib/unlang/catch_priv.h index 76133dfd14..22013116a3 100644 --- a/src/lib/unlang/catch_priv.h +++ b/src/lib/unlang/catch_priv.h @@ -31,8 +31,6 @@ extern "C" { typedef struct { unlang_group_t group; - - bool timeout; //!< are we catching a timeout bool catching[RLM_MODULE_NUMCODES]; } unlang_catch_t; diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index ddc9d59de0..5ad79d318a 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -2493,7 +2493,6 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C unlang_catch_t *ca; CONF_ITEM *prev; char const *name; - bool catching_timeout = false; static unlang_ext_t const ext = { .type = UNLANG_TYPE_CATCH, @@ -2507,7 +2506,6 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C if (!prev || !cf_item_is_section(prev)) { fail: cf_log_err(cs, "Found 'catch' section with no previous 'try'"); - print_url: cf_log_err(ci, DOC_KEYWORD_REF(catch)); return NULL; } @@ -2515,30 +2513,7 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C name = cf_section_name1(cf_item_to_section(prev)); fr_assert(name != NULL); - if (strcmp(name, "timeout") == 0) { - CONF_ITEM *next; - - name = cf_section_name2(cs); - if (!name || (strcmp(name, "timeout") != 0)) { - cf_log_err(cs, "Invalid 'catch' after a 'timeout' section"); - goto print_url; - } - - /* - * Check that the next section is NOT a "catch". - */ - next = cf_item_next(cf_parent(ci), ci); - while (next && cf_item_is_data(next)) next = cf_item_next(cf_parent(ci), next); - - if (next && cf_item_is_section(next) && - (strcmp(cf_section_name1(cf_item_to_section(next)), "catch") == 0)) { - cf_log_err(next, "Cannot have two 'catch' statements after a 'timeout' section"); - goto print_url; - } - - catching_timeout = true; - - } else if ((strcmp(name, "try") != 0) && (strcmp(name, "catch") != 0)) { + if ((strcmp(name, "try") != 0) && (strcmp(name, "catch") != 0)) { /* * The previous thing has to be a section. And it has to * be either a "try" or a "catch". @@ -2550,16 +2525,14 @@ static unlang_t *compile_catch(unlang_t *parent, unlang_compile_t *unlang_ctx, C if (!g) return NULL; c = unlang_group_to_generic(g); - ca = unlang_group_to_catch(g); - if (catching_timeout) { - ca->timeout = catching_timeout; /* * Want to log what we caught */ c->debug_name = c->name = talloc_typed_asprintf(c, "%s %s", cf_section_name1(cs), cf_section_name2(cs)); - } else if (!cf_section_name2(cs)) { + ca = unlang_group_to_catch(g); + if (!cf_section_name2(cs)) { /* * No arg2: catch errors */ diff --git a/src/lib/unlang/timeout.c b/src/lib/unlang/timeout.c index 6105baacfc..7b01523697 100644 --- a/src/lib/unlang/timeout.c +++ b/src/lib/unlang/timeout.c @@ -25,12 +25,14 @@ RCSID("$Id$") #include +#include #include "group_priv.h" #include "timeout_priv.h" +#include "mod_action.h" #include "unlang_priv.h" typedef struct { - bool success; + bool fired; int depth; fr_time_delta_t timeout; request_t *request; @@ -46,62 +48,50 @@ static void unlang_timeout_handler(UNUSED fr_timer_list_t *tl, UNUSED fr_time_t { unlang_frame_state_timeout_t *state = talloc_get_type_abort(ctx, unlang_frame_state_timeout_t); request_t *request = talloc_get_type_abort(state->request, request_t); - - RDEBUG("Timeout reached, signalling interpreter to cancel child section."); + char const *module; /* - * Has to be done BEFORE cancelling the frames, as one might be yielded. + * Don't log in the context of the request */ - unlang_interpret_mark_runnable(request); + module = request->module; + request->module = NULL; + RDEBUG("Timeout reached, signalling interpreter to cancel child section."); + request->module = module; /* - * Signal all lower frames to exit, but the request can keep running. + * Signal all the frames upto, but not including the timeout + * frame to cancel. + * + * unlang_timeout_resume_done then runs, and returns "timeout" */ unlang_stack_signal(request, FR_SIGNAL_CANCEL, state->depth); - state->success = false; + unlang_interpret_mark_runnable(request); + state->fired = true; RINDENT_RESTORE(request, state); if (!state->instruction) return; - if (unlang_interpret_push_instruction(request, state->instruction, RLM_MODULE_FAIL, true) < 0) { + /* + * Push something else onto the stack to execute. + */ + if (unlikely(unlang_interpret_push_instruction(request, state->instruction, RLM_MODULE_TIMEOUT, true) < 0)) { unlang_interpret_signal(request, FR_SIGNAL_CANCEL); /* also stops the request and does cleanups */ } } -static unlang_action_t unlang_timeout_resume_done(UNUSED rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) +static unlang_action_t unlang_timeout_resume_done(UNUSED rlm_rcode_t *p_result, UNUSED request_t *request, unlang_stack_frame_t *frame) { unlang_frame_state_timeout_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_timeout_t); - unlang_t *unlang; - - unlang = frame->instruction->next; /* * No timeout, we go to the next instruction. * * Unless the next instruction is a "catch timeout", in which case we skip it. */ - if (state->success) { - if (!unlang || (unlang->type != UNLANG_TYPE_CATCH)) { - return UNLANG_ACTION_CALCULATE_RESULT; - } - - /* - * We skip the "catch" section if there's no timeout. - */ - return frame_set_next(frame, unlang->next); - } + if (!state->fired) return UNLANG_ACTION_CALCULATE_RESULT; - RWDEBUG("Timeout exceeded"); - - /* - * If there's a next instruction, AND it's a "catch", then we catch the timeout. - */ - if (unlang && (unlang->type == UNLANG_TYPE_CATCH)) { - return frame_set_next(frame, unlang); - } - - return UNLANG_ACTION_FAIL; + RETURN_MODULE_TIMEOUT; } static unlang_action_t unlang_timeout_set(rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame) @@ -123,7 +113,6 @@ static unlang_action_t unlang_timeout_set(rlm_rcode_t *p_result, request_t *requ } frame_repeat(frame, unlang_timeout_resume_done); - state->success = true; return unlang_interpret_push_children(p_result, request, frame->result, UNLANG_NEXT_SIBLING); } @@ -151,7 +140,11 @@ static unlang_action_t unlang_timeout(rlm_rcode_t *p_result, request_t *request, g = unlang_generic_to_group(frame->instruction); gext = unlang_group_to_timeout(g); - state->depth = stack->depth; + /* + * +1 so we don't mark the timeout frame as cancelled, + * we want unlang_timeout_resume_done to be called. + */ + state->depth = stack->depth + 1; state->request = request; if (!gext->vpt) { @@ -238,11 +231,8 @@ int unlang_timeout_section_push(request_t *request, CONF_SECTION *cs, fr_time_de state->request = request; state->timeout = timeout; state->instruction = instruction; - state->success = true; - - when = fr_time_add(fr_time(), timeout); - if (fr_timer_at(state, unlang_interpret_event_list(request)->tl, &state->ev, when, + if (fr_timer_in(state, unlang_interpret_event_list(request)->tl, &state->ev, timeout, false, unlang_timeout_handler, state) < 0) { RPEDEBUG("Failed setting timeout for section %s", cf_section_name1(cs)); return -1; @@ -257,11 +247,11 @@ int unlang_timeout_section_push(request_t *request, CONF_SECTION *cs, fr_time_de void unlang_timeout_init(void) { unlang_register(UNLANG_TYPE_TIMEOUT, - &(unlang_op_t){ + &(unlang_op_t){ .name = "timeout", .interpret = unlang_timeout, - .flag = UNLANG_OP_FLAG_DEBUG_BRACES, + .flag = UNLANG_OP_FLAG_DEBUG_BRACES | UNLANG_OP_FLAG_RCODE_SET, .frame_state_size = sizeof(unlang_frame_state_timeout_t), .frame_state_type = "unlang_frame_state_timeout_t", - }); + }); } diff --git a/src/tests/keywords/timeout-catch b/src/tests/keywords/timeout-catch new file mode 100644 index 0000000000..4e43ed912c --- /dev/null +++ b/src/tests/keywords/timeout-catch @@ -0,0 +1,15 @@ +# +# PRE: timeout +# + +# +# @todo - we have to add a leading '0' here, otherwise cf_file.c complains +# +try { + timeout 0.1s { + delay_10s + } +} +catch timeout { + success +} diff --git a/src/tests/keywords/timeout.attrs b/src/tests/keywords/timeout.attrs new file mode 100644 index 0000000000..3e34f0b728 --- /dev/null +++ b/src/tests/keywords/timeout.attrs @@ -0,0 +1,11 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = "bob" +User-Password = "hello" + +# +# Expected answer +# +Packet-Type == Do-Not-Respond