From: Arran Cudbard-Bell Date: Fri, 2 Apr 2021 17:07:25 +0000 (+0100) Subject: Run triggers synchronously in certain situations X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b97591c3fb4b7c0588edf83265a8b75c49fb6a6b;p=thirdparty%2Ffreeradius-server.git Run triggers synchronously in certain situations --- diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index 0088335d7ee..b1782002fb4 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -838,7 +838,7 @@ int main(int argc, char *argv[]) } } - trigger_exec(NULL, NULL, "server.start", false, NULL); + trigger_exec(NULL, NULL, NULL, "server.start", false, NULL); /* * Inform the parent (who should still be waiting) that the rest of @@ -923,8 +923,8 @@ int main(int argc, char *argv[]) * Fire signal and stop triggers after ignoring SIGTERM, so handlers are * not killed with the rest of the process group, below. */ - if (status == 2) trigger_exec(NULL, NULL, "server.signal.term", true, NULL); - trigger_exec(NULL, NULL, "server.stop", false, NULL); + if (status == 2) trigger_exec(NULL, NULL, NULL, "server.signal.term", true, NULL); + trigger_exec(NULL, NULL, NULL, "server.stop", false, NULL); /* * Stop the scheduler, this signals the network and worker threads diff --git a/src/lib/io/listen.h b/src/lib/io/listen.h index 47a3ce87f4c..4c184d0941c 100644 --- a/src/lib/io/listen.h +++ b/src/lib/io/listen.h @@ -64,7 +64,7 @@ struct fr_async_s { uint32_t priority; //!< higher == higher priority }; -#define request_is_external(_x) (_x->async->listen != NULL) +#define request_is_external(_x) (_x->async && (_x->async->listen != NULL)) #define request_is_internal(_x) (!request_is_external(_x)) int fr_io_listen_free(fr_listen_t *li); diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c index 86b46a54c8d..bbd7e6446ca 100644 --- a/src/lib/io/worker.c +++ b/src/lib/io/worker.c @@ -883,6 +883,11 @@ void fr_worker_destroy(fr_worker_t *worker) // WORKER_VERIFY; + /* + * Stop any new requests running with this interpreter + */ + unlang_interpret_set_thread_default(NULL); + /* * Destroy all of the active requests. These are ones * which are still waiting for timers or file descriptor diff --git a/src/lib/server/connection.c b/src/lib/server/connection.c index 95db82f86d6..ae687bef658 100644 --- a/src/lib/server/connection.c +++ b/src/lib/server/connection.c @@ -29,11 +29,11 @@ typedef struct fr_connection_s fr_connection_t; #define _CONNECTION_PRIVATE 1 #include -#include -#include #include +#include #include +#include #include #include #include @@ -126,7 +126,8 @@ struct fr_connection_s { #define CONN_TRIGGER(_state) do { \ if (conn->pub.triggers) { \ - trigger_exec(NULL, NULL, fr_table_str_by_value(fr_connection_trigger_names, _state, ""), true, NULL); \ + trigger_exec(unlang_interpret_get_thread_default(), \ + NULL, NULL, fr_table_str_by_value(fr_connection_trigger_names, _state, ""), true, NULL); \ } \ } while (0) @@ -164,13 +165,13 @@ typedef enum { } connection_dsignal_t; static fr_table_num_ordered_t const connection_dsignals[] = { - { L("INIT"), CONNECTION_DSIGNAL_INIT }, + { L("INIT"), CONNECTION_DSIGNAL_INIT }, { L("CONNECTING"), CONNECTION_DSIGNAL_CONNECTED }, { L("RECONNECT-FAILED"), CONNECTION_DSIGNAL_RECONNECT_FAILED }, { L("RECONNECT-EXPIRED"), CONNECTION_DSIGNAL_RECONNECT_EXPIRED }, { L("SHUTDOWN"), CONNECTION_DSIGNAL_SHUTDOWN }, - { L("HALT"), CONNECTION_DSIGNAL_HALT }, - { L("FREE"), CONNECTION_DSIGNAL_FREE } + { L("HALT"), CONNECTION_DSIGNAL_HALT }, + { L("FREE"), CONNECTION_DSIGNAL_FREE } }; static size_t connection_dsignals_len = NUM_ELEMENTS(connection_dsignals); diff --git a/src/lib/server/exec.h b/src/lib/server/exec.h index eb08c0b0e5a..37bc08cc4ee 100644 --- a/src/lib/server/exec.h +++ b/src/lib/server/exec.h @@ -61,8 +61,6 @@ int fr_exec_nowait(request_t *request, fr_value_box_list_t *vb_list, fr_pair_lis int fr_exec_wait_start(pid_t *pid_p, int *stdin_fd, int *stdout_fd, int *stderr_fd, request_t *request, fr_value_box_list_t *vb_list, fr_pair_list_t *env_pairs); -void fr_exec_waitpid(pid_t pid); - #ifdef __cplusplus } #endif diff --git a/src/lib/server/exfile.c b/src/lib/server/exfile.c index 4f1c63335c0..7c6668fe279 100644 --- a/src/lib/server/exfile.c +++ b/src/lib/server/exfile.c @@ -94,7 +94,7 @@ static inline void exfile_trigger_exec(exfile_t *ef, request_t *request, exfile_ fr_dcursor_prepend(&cursor, vp); snprintf(name, sizeof(name), "%s.%s", ef->trigger_prefix, name_suffix); - trigger_exec(request, ef->conf, name, false, &args); + trigger_exec(unlang_interpret_get_thread_default(), request, ef->conf, name, false, &args); fr_pair_list_free(&args); } diff --git a/src/lib/server/main_loop.c b/src/lib/server/main_loop.c index 6d4d91b618c..e7a272bbdde 100644 --- a/src/lib/server/main_loop.c +++ b/src/lib/server/main_loop.c @@ -131,7 +131,7 @@ static void main_loop_signal_process(int flag) last_hup = when; - trigger_exec(NULL, NULL, "server.signal.hup", true, NULL); + trigger_exec(unlang_interpret_get_thread_default(), NULL, NULL, "server.signal.hup", true, NULL); fr_event_loop_exit(event_list, 0x80); } } diff --git a/src/lib/server/pool.c b/src/lib/server/pool.c index 6bc4a26910e..1ac92c88810 100644 --- a/src/lib/server/pool.c +++ b/src/lib/server/pool.c @@ -256,7 +256,7 @@ static inline void fr_pool_trigger_exec(fr_pool_t *pool, request_t *request, cha if (!pool->triggers_enabled) return; snprintf(name, sizeof(name), "%s.%s", pool->trigger_prefix, event); - trigger_exec(request, pool->cs, name, true, &pool->trigger_args); + trigger_exec(unlang_interpret_get_thread_default(), request, pool->cs, name, true, &pool->trigger_args); } /** Find a connection handle in the connection list diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c index 67464b35e67..a79750ab60d 100644 --- a/src/lib/server/trigger.c +++ b/src/lib/server/trigger.c @@ -184,6 +184,8 @@ typedef struct { char *name; xlat_exp_t *xlat; fr_value_box_list_t args; + bool synchronous; + pid_t pid; //!< for synchronous execution. } fr_trigger_t; @@ -198,11 +200,28 @@ static unlang_action_t trigger_resume(rlm_rcode_t *p_result, UNUSED int *priorit } /* - * Execute the program without waiting for argss. + * Execute the program and wait for it to finish before + * continuing. This blocks the executing thread. */ - if (fr_exec_nowait(request, &trigger->args, NULL) < 0) { - RPERROR("Failed running trigger %s", trigger->name); - RETURN_MODULE_FAIL; + if (trigger->synchronous) { + if (fr_exec_wait_start(&trigger->pid, NULL, NULL, NULL, request, &trigger->args, NULL) < 0) { + RPERROR("Failed running trigger %s", trigger->name); + RETURN_MODULE_FAIL; + } + /* + * Wait for the trigger to finish + * + * FIXME - We really need to log stdout/stderr + */ + waitpid(&trigger->pid, NULL, 0); + /* + * Execute the program without waiting for the result. + */ + } else { + if (fr_exec_nowait(request, &trigger->args, NULL) < 0) { + RPERROR("Failed running trigger %s", trigger->name); + RETURN_MODULE_FAIL; + } } RETURN_MODULE_OK; @@ -225,19 +244,24 @@ static unlang_action_t trigger_run(rlm_rcode_t *p_result, UNUSED int *priority, * * @note Calls to this function will be ignored if #trigger_exec_init has not been called. * - * @param request The current request. - * @param cs to search for triggers in. - * If cs is not NULL, the portion after the last '.' in name is used for the trigger. - * If cs is NULL, the entire name is used to find the trigger in the global trigger - * section. - * @param name the path relative to the global trigger section ending in the trigger name - * e.g. module.ldap.pool.start. - * @param rate_limit whether to rate limit triggers. - * @param args to make available via the @verbatim %(trigger:) @endverbatim xlat. - * @return - 0 on success. - * - -1 on failure. + * @param[in] intp Interpreter to run the trigger with. If this is NULL the + * trigger will be executed synchronously. + * + * @param[in] request The current request. + * @param[in] cs to search for triggers in. + * If cs is not NULL, the portion after the last '.' in name is used for the trigger. + * If cs is NULL, the entire name is used to find the trigger in the global trigger + * section. + * @param[in] name the path relative to the global trigger section ending in the trigger name + * e.g. module.ldap.pool.start. + * @param[in] rate_limit whether to rate limit triggers. + * @param[in] args to make available via the @verbatim %(trigger:) @endverbatim xlat. + * @return + * - 0 on success. + * - -1 on failure. */ -int trigger_exec(request_t *request, CONF_SECTION const *cs, char const *name, bool rate_limit, fr_pair_list_t *args) +int trigger_exec(unlang_interpret_t *intp, request_t *request, + CONF_SECTION const *cs, char const *name, bool rate_limit, fr_pair_list_t *args) { CONF_SECTION const *subcs; @@ -399,18 +423,33 @@ int trigger_exec(request_t *request, CONF_SECTION const *cs, char const *name, b } /* - * Always run triggers in the default interpreter + * If we're not running it locally use the default + * interpreter for the thread. */ - unlang_interpret_set(child, unlang_interpret_get_thread_default()); - - if (unlang_subrequest_child_push_and_detach(child) < 0) goto error; + if (intp) { + unlang_interpret_set(child, intp); + if (unlang_subrequest_child_push_and_detach(child) < 0) { + error: + ROPTIONAL(RPEDEBUG, PERROR, "Running trigger failed"); + talloc_free(child); + return -1; + } + } if (unlang_interpret_push_function(child, trigger_run, trigger_resume, - NULL, UNLANG_TOP_FRAME, trigger) < 0) { - error: - ROPTIONAL(RPEDEBUG, PERROR, "Running trigger failed"); + NULL, UNLANG_TOP_FRAME, trigger) < 0) goto error; + + if (!intp) { + /* + * Wait for the exec to finish too, + * so where there are global events + * the child processes don't race + * with something like the server + * shutting down. + */ + trigger->synchronous = true; + unlang_interpret_synchronous(child); talloc_free(child); - return -1; } /* diff --git a/src/lib/server/trigger.h b/src/lib/server/trigger.h index 12ce8e08b02..f83ab1a2664 100644 --- a/src/lib/server/trigger.h +++ b/src/lib/server/trigger.h @@ -30,8 +30,9 @@ extern "C" { #endif #include -#include #include +#include +#include #include #include @@ -41,9 +42,10 @@ xlat_action_t trigger_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *reques int trigger_exec_init(CONF_SECTION const *cs); -int trigger_exec(request_t *request, CONF_SECTION const *cs, +int trigger_exec(unlang_interpret_t *intp, + request_t *request, CONF_SECTION const *cs, char const *name, bool quench, fr_pair_list_t *args) - CC_HINT(nonnull (3)); + CC_HINT(nonnull (4)); void trigger_exec_free(void); diff --git a/src/lib/server/trunk.c b/src/lib/server/trunk.c index d7863a36fbe..ff6713d596d 100644 --- a/src/lib/server/trunk.c +++ b/src/lib/server/trunk.c @@ -393,7 +393,8 @@ static size_t fr_trunk_connection_events_len = NUM_ELEMENTS(fr_trunk_connection_ #define CONN_TRIGGER(_state) do { \ if (trunk->pub.triggers) { \ - trigger_exec(NULL, NULL, fr_table_str_by_value(fr_trunk_conn_trigger_names, _state, \ + trigger_exec(unlang_interpret_get_thread_default(), \ + NULL, NULL, fr_table_str_by_value(fr_trunk_conn_trigger_names, _state, \ ""), true, NULL); \ } \ } while (0) @@ -423,7 +424,8 @@ void trunk_request_state_log_entry_add(char const *function, int line, #define REQUEST_TRIGGER(_state) do { \ if (trunk->pub.triggers) { \ - trigger_exec(NULL, NULL, fr_table_str_by_value(fr_trunk_req_trigger_names, _state, \ + trigger_exec(unlang_interpret_get_thread_default(), \ + NULL, NULL, fr_table_str_by_value(fr_trunk_req_trigger_names, _state, \ ""), true, NULL); \ } \ } while (0) diff --git a/src/lib/unlang/function.c b/src/lib/unlang/function.c index 81c3bb97921..0961d7c9a3e 100644 --- a/src/lib/unlang/function.c +++ b/src/lib/unlang/function.c @@ -32,6 +32,7 @@ RCSID("$Id$") * Some functions differ mainly in their parsing */ typedef struct { + bool done_func; //!< Set to true after func has been executed. unlang_function_t func; //!< To call when going down the stack. unlang_function_t repeat; //!< To call when going back up the stack. unlang_function_signal_t signal; //!< Signal function to call. @@ -92,8 +93,9 @@ static unlang_action_t unlang_function_call(rlm_rcode_t *p_result, request_t *re */ caller = request->module; request->module = NULL; - if (!is_repeatable(frame)) { + if (!state->done_func) { ua = state->func(p_result, &frame->priority, request, state->uctx); + state->done_func = true; } else { ua = state->repeat(p_result, &frame->priority, request, state->uctx); } diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c index d7ed314a199..0eacf9eee4b 100644 --- a/src/lib/unlang/interpret.c +++ b/src/lib/unlang/interpret.c @@ -1196,11 +1196,12 @@ unlang_interpret_t *unlang_interpret_get(request_t *request) /** Set the default interpreter for this thread * - */ void unlang_interpret_set_thread_default(unlang_interpret_t *intp) { - intp_thread_default = talloc_get_type_abort(intp, unlang_interpret_t); + if (intp) (void)talloc_get_type_abort(intp, unlang_interpret_t); + + intp_thread_default = intp; } /** Get the default interpreter for this thread diff --git a/src/modules/proto_bfd/proto_bfd.c b/src/modules/proto_bfd/proto_bfd.c index d2b9e270170..da13be305ad 100644 --- a/src/modules/proto_bfd/proto_bfd.c +++ b/src/modules/proto_bfd/proto_bfd.c @@ -407,7 +407,7 @@ static void bfd_trigger(bfd_state_t *session) bfd_request(session, request, &packet); - trigger_exec(request, NULL, buffer, false, NULL); + trigger_exec(unlang_interpret_get_thread_default(), request, NULL, buffer, false, NULL); } @@ -1329,7 +1329,7 @@ static int bfd_process(bfd_state_t *session, bfd_packet_t *bfd) bfd_request(session, &request, &packet); - trigger_exec(&request, NULL, "server.bfd.warn", false, NULL); + trigger_exec(unlang_interpret_get_thread_default(), &request, NULL, "server.bfd.warn", false, NULL); }