From 08559f724b3ba14e784e3ec0a6ee5ea2f25251d3 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Sun, 12 May 2024 10:48:51 -0600 Subject: [PATCH] Move unlang, request, and xlat initialisation to atexit handlers The atexit handlers just run in the reverse order to which they were created, so unless the code is doing something weird and incestuous it makes cleanup much simpler --- src/bin/radiusd.c | 5 ---- src/bin/unit_test_attribute.c | 4 --- src/bin/unit_test_map.c | 4 --- src/bin/unit_test_module.c | 5 ---- src/lib/server/base.c | 10 ------- src/lib/server/pair_server_tests.c | 7 ----- src/lib/server/request.c | 16 ++++++++--- src/lib/server/tmpl_dcursor_tests.c | 7 ----- src/lib/unlang/base.c | 32 +++++++++------------- src/lib/unlang/base.h | 2 -- src/lib/unlang/xlat.h | 2 +- src/lib/unlang/xlat_builtin.c | 42 ++++++++++++++++++----------- 12 files changed, 53 insertions(+), 83 deletions(-) diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index 45085b2123..62d71ed816 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -1091,11 +1091,6 @@ cleanup: server_free(); - /* - * Free any resources used by the unlang interpreter. - */ - unlang_global_free(); - #ifdef WITH_TLS fr_openssl_free(); /* Cleanup any memory alloced by OpenSSL and placed into globals */ #endif diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c index 15b404eec4..8ab29b07f1 100644 --- a/src/bin/unit_test_attribute.c +++ b/src/bin/unit_test_attribute.c @@ -3938,10 +3938,6 @@ cleanup: EXIT_WITH_FAILURE; } - unlang_global_free(); - - request_global_free(); - if (receipt_file && (ret == EXIT_SUCCESS) && (fr_touch(NULL, receipt_file, 0644, true, 0755) <= 0)) { fr_perror("unit_test_attribute"); EXIT_WITH_FAILURE; diff --git a/src/bin/unit_test_map.c b/src/bin/unit_test_map.c index 847ac5eb8f..79b8938bdc 100644 --- a/src/bin/unit_test_map.c +++ b/src/bin/unit_test_map.c @@ -31,8 +31,6 @@ RCSID("$Id$") #include -#include - #ifdef HAVE_GETOPT_H # include #endif @@ -280,8 +278,6 @@ cleanup: */ fr_atexit_thread_trigger_all(); - request_global_free(); - /* * Free any autoload dictionaries */ diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c index d916d8f01a..83c15f9be0 100644 --- a/src/bin/unit_test_module.c +++ b/src/bin/unit_test_module.c @@ -1205,11 +1205,6 @@ cleanup: server_free(); - /* - * Free any resources used by the unlang interpreter. - */ - unlang_global_free(); - /* * Virtual servers need to be freed before modules * as state entries containing data with module-specific diff --git a/src/lib/server/base.c b/src/lib/server/base.c index b96697d982..43f663337f 100644 --- a/src/lib/server/base.c +++ b/src/lib/server/base.c @@ -125,22 +125,12 @@ void server_free(void) */ password_free(); - /* - * The only maps remaining are the ones registered by the server core. - */ - map_proc_free(); - /* * Now we're sure no more triggers can fire, free the * trigger tree. */ trigger_exec_free(); - /* - * Free the internal dictionaries the request uses - */ - request_global_free(); - /* * Free the internal dictionaries the tmpl code uses */ diff --git a/src/lib/server/pair_server_tests.c b/src/lib/server/pair_server_tests.c index 1f5c8729ec..8bd74fc53c 100644 --- a/src/lib/server/pair_server_tests.c +++ b/src/lib/server/pair_server_tests.c @@ -22,9 +22,7 @@ */ static void test_init(void); -static void test_free(void); # define TEST_INIT test_init() -# define TEST_FINI test_free() #include #include @@ -74,11 +72,6 @@ static void test_init(void) if (request_global_init() < 0) goto error; } -static void test_free(void) -{ - request_global_free(); -} - static request_t *request_fake_alloc(void) { request_t *request; diff --git a/src/lib/server/request.c b/src/lib/server/request.c index 39d5b16168..be1b85abc1 100644 --- a/src/lib/server/request.c +++ b/src/lib/server/request.c @@ -26,6 +26,7 @@ RCSID("$Id$") #include #include +#include static request_init_args_t default_args; @@ -695,7 +696,13 @@ int request_detach(request_t *child) return 0; } -int request_global_init(void) +static int _request_global_free(UNUSED void *uctx) +{ + fr_dict_autofree(request_dict); + return 0; +} + +static int _request_global_init(UNUSED void *uctx) { if (fr_dict_autoload(request_dict) < 0) { PERROR("%s", __FUNCTION__); @@ -706,13 +713,14 @@ int request_global_init(void) fr_dict_autofree(request_dict); return -1; } - return 0; } -void request_global_free(void) +int request_global_init(void) { - fr_dict_autofree(request_dict); + int ret; + fr_atexit_global_once_ret(&ret, _request_global_init, _request_global_free, NULL); + return ret; } #ifdef WITH_VERIFY_PTR diff --git a/src/lib/server/tmpl_dcursor_tests.c b/src/lib/server/tmpl_dcursor_tests.c index 1dcc5f7ffc..14725bec42 100644 --- a/src/lib/server/tmpl_dcursor_tests.c +++ b/src/lib/server/tmpl_dcursor_tests.c @@ -1,7 +1,5 @@ static void test_init(void); -static void test_free(void); # define TEST_INIT test_init() -# define TEST_FINI test_free() #include #include @@ -36,11 +34,6 @@ static void test_init(void) if (request_global_init() < 0) goto error; } -static void test_free(void) -{ - request_global_free(); -} - static request_t *request_fake_alloc(void) { request_t *request; diff --git a/src/lib/unlang/base.c b/src/lib/unlang/base.c index e5555ed76b..fc841c11f4 100644 --- a/src/lib/unlang/base.c +++ b/src/lib/unlang/base.c @@ -26,8 +26,6 @@ RCSID("$Id$") #include "unlang_priv.h" -static uint32_t instance_count; - /** Different operations the interpreter can execute */ unlang_op_t unlang_ops[UNLANG_TYPE_MAX]; @@ -71,20 +69,23 @@ void unlang_register(int type, unlang_op_t *op) static TALLOC_CTX *unlang_ctx = NULL; -int unlang_global_init(void) +static int _unlang_global_free(UNUSED void *uctx) { - if (instance_count > 0) { - instance_count++; - return 0; - } + unlang_subrequest_op_free(); + TALLOC_FREE(unlang_ctx); + + return 0; +} +static int _unlang_global_init(UNUSED void *uctx) +{ unlang_ctx = talloc_init("unlang"); if (!unlang_ctx) return -1; /* * Explicitly initialise the xlat tree, and perform dictionary lookups. */ - if (xlat_global_init(unlang_ctx) < 0) { + if (xlat_global_init() < 0) { fail: TALLOC_FREE(unlang_ctx); @@ -126,19 +127,12 @@ int unlang_global_init(void) unlang_try_init(); unlang_catch_init(); - instance_count++; - return 0; } -void unlang_global_free(void) +int unlang_global_init(void) { - if (--instance_count > 0) return; - - unlang_subrequest_op_free(); - TALLOC_FREE(unlang_ctx); - - memset(unlang_ops, 0, sizeof(unlang_ops)); - - xlat_global_free(); + int ret; + fr_atexit_global_once_ret(&ret, _unlang_global_init, _unlang_global_free, NULL); + return ret; } diff --git a/src/lib/unlang/base.h b/src/lib/unlang/base.h index 000b670c9d..12c3905a47 100644 --- a/src/lib/unlang/base.h +++ b/src/lib/unlang/base.h @@ -38,8 +38,6 @@ bool unlang_section(CONF_SECTION *cs); int unlang_global_init(void); -void unlang_global_free(void); - int unlang_thread_instantiate(TALLOC_CTX *ctx) CC_HINT(nonnull); #ifdef WITH_PERF diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h index 729592a277..3e0012cdb0 100644 --- a/src/lib/unlang/xlat.h +++ b/src/lib/unlang/xlat.h @@ -499,7 +499,7 @@ xlat_action_t unlang_xlat_yield(request_t *request, * xlat_builtin.c */ int xlat_protocols_register(void); -int xlat_global_init(TALLOC_CTX *ctx); +int xlat_global_init(void); void xlat_global_free(void); #ifdef __cplusplus diff --git a/src/lib/unlang/xlat_builtin.c b/src/lib/unlang/xlat_builtin.c index 1236b48531..2bd688163c 100644 --- a/src/lib/unlang/xlat_builtin.c +++ b/src/lib/unlang/xlat_builtin.c @@ -60,6 +60,7 @@ RCSID("$Id$") #include static char const hextab[] = "0123456789abcdef"; +static TALLOC_CTX *xlat_ctx; /** Return a VP from the specified request. * @@ -3905,6 +3906,17 @@ int xlat_protocols_register(void) return 0; } +/** De-register all xlat functions we created + * + */ +static int _xlat_global_free(UNUSED void *uctx) +{ + TALLOC_FREE(xlat_ctx); + xlat_func_free(); + xlat_eval_free(); + + return 0; +} /** Global initialisation for xlat * @@ -3916,10 +3928,13 @@ int xlat_protocols_register(void) * * @hidecallgraph */ -int xlat_global_init(TALLOC_CTX *ctx) +static int _xlat_global_init(UNUSED void *uctx) { xlat_t *xlat; + xlat_ctx = talloc_init("xlat"); + if (!xlat_ctx) return -1; + if (xlat_func_init() < 0) return -1; /* @@ -3937,7 +3952,7 @@ int xlat_global_init(TALLOC_CTX *ctx) */ #define XLAT_REGISTER_ARGS(_xlat, _func, _return_type, _args) \ do { \ - if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ + if (unlikely((xlat = xlat_func_register(xlat_ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ xlat_func_args_set(xlat, _args); \ xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \ } while (0) @@ -3967,7 +3982,7 @@ do { \ #undef XLAT_REGISTER_ARGS #define XLAT_REGISTER_ARGS(_xlat, _func, _return_type, _args) \ do { \ - if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ + if (unlikely((xlat = xlat_func_register(xlat_ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ xlat_func_args_set(xlat, _args); \ xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL); \ } while (0) @@ -3991,10 +4006,10 @@ do { \ XLAT_REGISTER_ARGS("base64.encode", xlat_func_base64_encode, FR_TYPE_STRING, xlat_func_base64_encode_arg); XLAT_REGISTER_ARGS("base64.decode", xlat_func_base64_decode, FR_TYPE_OCTETS, xlat_func_base64_decode_arg); - if (unlikely((xlat = xlat_func_register(ctx, "untaint", xlat_func_untaint, FR_TYPE_VOID)) == NULL)) return -1; + if (unlikely((xlat = xlat_func_register(xlat_ctx, "untaint", xlat_func_untaint, FR_TYPE_VOID)) == NULL)) return -1; xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); - if (unlikely((xlat = xlat_func_register(ctx, "taint", xlat_func_taint, FR_TYPE_VOID)) == NULL)) return -1; + if (unlikely((xlat = xlat_func_register(xlat_ctx, "taint", xlat_func_taint, FR_TYPE_VOID)) == NULL)) return -1; xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); /* @@ -4002,7 +4017,7 @@ do { \ */ #define XLAT_REGISTER_MONO(_xlat, _func, _return_type, _arg) \ do { \ - if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ + if (unlikely((xlat = xlat_func_register(xlat_ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ xlat_func_mono_set(xlat, _arg); \ xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \ } while (0) @@ -4014,7 +4029,7 @@ do { \ XLAT_REGISTER_MONO("md4", xlat_func_md4, FR_TYPE_OCTETS, xlat_func_md4_arg); XLAT_REGISTER_MONO("md5", xlat_func_md5, FR_TYPE_OCTETS, xlat_func_md5_arg); #if defined(HAVE_REGEX_PCRE) || defined(HAVE_REGEX_PCRE2) - if (unlikely((xlat = xlat_func_register(ctx, "regex", xlat_func_regex, FR_TYPE_STRING)) == NULL)) return -1; + if (unlikely((xlat = xlat_func_register(xlat_ctx, "regex", xlat_func_regex, FR_TYPE_STRING)) == NULL)) return -1; xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL); #endif XLAT_REGISTER_MONO("sha1", xlat_func_sha1, FR_TYPE_OCTETS, xlat_func_sha_arg); @@ -4048,7 +4063,7 @@ do { \ #undef XLAT_REGISTER_MONO #define XLAT_REGISTER_MONO(_xlat, _func, _return_type, _arg) \ do { \ - if (unlikely((xlat = xlat_func_register(ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ + if (unlikely((xlat = xlat_func_register(xlat_ctx, _xlat, _func, _return_type)) == NULL)) return -1; \ xlat_func_mono_set(xlat, _arg); \ xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_INTERNAL); \ } while (0) @@ -4059,12 +4074,9 @@ do { \ return xlat_register_expressions(); } -/** De-register all xlat functions we created - * - */ -void xlat_global_free(void) +int xlat_global_init(void) { - xlat_func_free(); - - xlat_eval_free(); + int ret; + fr_atexit_global_once_ret(&ret, _xlat_global_init, _xlat_global_free, NULL); + return ret; } -- 2.47.3