Run triggers synchronously in certain situations

diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c
index 0088335d7eedadcfb5b4ed9c378768a658301347..b1782002fb4bee4540bd8866753ee7ae559bdfdc 100644 (file)
--- 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 47a3ce87f4c8f426b4c6d08c762e3a56eafd782f..4c184d0941ccd905108f65b356d07df456e61cb1 100644 (file)
--- 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 86b46a54c8d46c169bf0cfe5b69e6a3d6dbf99b8..bbd7e6446ca6828466302eafc62a7cf615e8b418 100644 (file)
--- 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 95db82f86d68acb835cdc0683eca8b4e600d164c..ae687bef65870b44b5dce2ce568fa4a0cc0f6f66 100644 (file)
--- 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 <freeradius-devel/server/connection.h>
 
-#include <freeradius-devel/server/log.h>
-#include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/server/cond_eval.h>
+#include <freeradius-devel/server/log.h>
 #include <freeradius-devel/server/trigger.h>
 
+#include <freeradius-devel/util/debug.h>
 #include <freeradius-devel/util/base.h>
 #include <freeradius-devel/util/event.h>
 #include <freeradius-devel/util/talloc.h>
@@ -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, "<INVALID>"), true, NULL); \
+               trigger_exec(unlang_interpret_get_thread_default(), \
+                            NULL, NULL, fr_table_str_by_value(fr_connection_trigger_names, _state, "<INVALID>"), 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 eb08c0b0e5aed1eb671b001084aec05117aa506b..37bc08cc4ee746124ed16eefd36704b25a21f10e 100644 (file)
--- 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 4f1c63335c03b6efdd236902bb4fa2f248d310ba..7c6668fe279947e0a3bd4daee01143e07d317bb4 100644 (file)
--- 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 6d4d91b618c68c3d1fcc4ecdfeaef60acb909ead..e7a272bbdde41123370f0e8c222b7dcd74f304ea 100644 (file)
--- 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 6bc4a26910e685f4642d26266bac04cc2f1e4dc8..1ac92c888106ee06f35331477e575ae9f137c817 100644 (file)
--- 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 67464b35e671f1ad2d5d069dd0108aa69b755808..a79750ab60db6b21adf6d957498a1c6d9bf466a8 100644 (file)
--- 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:<arg>) @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:<arg>) @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 12ce8e08b027af18e1d3efa5d5bc02e26d03a034..f83ab1a266498dfcbd572eb7205a6d38a35c1fd4 100644 (file)
--- a/src/lib/server/trigger.h
+++ b/src/lib/server/trigger.h
@@ -30,8 +30,9 @@ extern "C" {
 #endif
 
 #include <freeradius-devel/server/cf_util.h>
-#include <freeradius-devel/server/request.h>
 #include <freeradius-devel/server/module.h>
+#include <freeradius-devel/server/request.h>
+#include <freeradius-devel/unlang/interpret.h>
 #include <freeradius-devel/util/pair.h>
 #include <freeradius-devel/util/talloc.h>
 
@@ -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 d7863a36fbe8874ec1ad61b0860a693174a84169..ff6713d596dfbbb15d99aa480cef51b54c9184a8 100644 (file)
--- 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, \
                                                               "<INVALID>"), 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, \
                                                               "<INVALID>"), true, NULL); \
        } \
 } while (0)

diff --git a/src/lib/unlang/function.c b/src/lib/unlang/function.c
index 81c3bb9792148650c132d047a0fa0bd24a71f072..0961d7c9a3e8a557e64a00f24c175388f24f8189 100644 (file)
--- 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 d7ed314a19937ea6b95ee02747aa665c022801b7..0eacf9eee4b0bf826f55b4d95fd92cbf64f31856 100644 (file)
--- 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 d2b9e2701703c7c2516a3eb04ae24ac1dd8d106e..da13be305ada275175487ed7ff588870b1705a9c 100644 (file)
--- 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);
        }