From: Arran Cudbard-Bell Date: Fri, 13 Mar 2020 01:10:51 +0000 (-0400) Subject: thread_local: Make sure that the thread_local atexit handler is called _last_ X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5feee044a3b91e4be60268a1f83b4e67a48d32e6;p=thirdparty%2Ffreeradius-server.git thread_local: Make sure that the thread_local atexit handler is called _last_ --- diff --git a/src/bin/radclient.c b/src/bin/radclient.c index 417e03e743c..06f76dc8058 100644 --- a/src/bin/radclient.c +++ b/src/bin/radclient.c @@ -1178,7 +1178,7 @@ int main(int argc, char **argv) int parallel = 1; rc_request_t *this; int force_af = AF_UNSPEC; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; /* * It's easier having two sets of flags to set the @@ -1188,6 +1188,13 @@ int main(int argc, char **argv) fr_debug_lvl = 0; fr_log_fp = stdout; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { fr_perror("radclient"); diff --git a/src/bin/radict.c b/src/bin/radict.c index 23c766a48a2..832e93a0ad7 100644 --- a/src/bin/radict.c +++ b/src/bin/radict.c @@ -197,7 +197,14 @@ int main(int argc, char *argv[]) bool found = false; bool export = false; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; + + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index 97baab82fdf..81a2ed41e0f 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -223,6 +223,10 @@ int main(int argc, char *argv[]) #ifndef NDEBUG fr_time_delta_t exit_after = 0; #endif + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); /* * Setup talloc callbacks so we get useful errors diff --git a/src/bin/radsniff.c b/src/bin/radsniff.c index fb08489a9ff..6c042137b78 100644 --- a/src/bin/radsniff.c +++ b/src/bin/radsniff.c @@ -2210,13 +2210,20 @@ int main(int argc, char *argv[]) int c; char const *raddb_dir = RADDBDIR; char const *dict_dir = DICTDIR; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; rs_stats_t *stats; fr_debug_lvl = 1; fr_log_fp = stdout; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + /* * Useful if using radsniff as a long running stats daemon */ diff --git a/src/bin/radsnmp.c b/src/bin/radsnmp.c index e3e49df331c..63d16572e24 100644 --- a/src/bin/radsnmp.c +++ b/src/bin/radsnmp.c @@ -890,7 +890,7 @@ int main(int argc, char **argv) radsnmp_conf_t *conf; int ret; int sockfd; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; conf = talloc_zero(autofree, radsnmp_conf_t); conf->proto = IPPROTO_UDP; @@ -900,6 +900,13 @@ int main(int argc, char **argv) conf->timeout = fr_time_delta_from_sec(3); conf->retries = 5; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { fr_perror("radsnmp"); diff --git a/src/bin/radwho.c b/src/bin/radwho.c index 55643e863fe..f725acbc445 100644 --- a/src/bin/radwho.c +++ b/src/bin/radwho.c @@ -197,11 +197,17 @@ int main(int argc, char **argv) uint32_t nas_ip_address = INADDR_NONE; int zap = 0; fr_dict_t *dict = NULL; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; char const *p; main_config_t *config; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c index 76cac1d4c25..5dcacb6d546 100644 --- a/src/bin/unit_test_attribute.c +++ b/src/bin/unit_test_attribute.c @@ -2348,7 +2348,7 @@ int main(int argc, char *argv[]) int *inst = &c; CONF_SECTION *cs; int ret = EXIT_SUCCESS; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; dl_module_loader_t *dl_modules = NULL; bool exit_now = false; @@ -2362,6 +2362,13 @@ int main(int argc, char *argv[]) bool do_commands = false; bool do_usage = false; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { fr_perror("unit_test_attribute"); diff --git a/src/bin/unit_test_map.c b/src/bin/unit_test_map.c index e9c15eacdea..780fe89949f 100644 --- a/src/bin/unit_test_map.c +++ b/src/bin/unit_test_map.c @@ -85,6 +85,11 @@ static int process_file(char const *filename) .allow_foreign = true /* Because we don't know what protocol we're operating with */ }; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + config = main_config_alloc(NULL); if (!config) { fprintf(stderr, "Failed allocating main config"); @@ -154,7 +159,14 @@ int main(int argc, char *argv[]) fr_dict_t *dict = NULL; char const *receipt_file = NULL; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; + + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c index b6b561d6c51..6e85b34fe98 100644 --- a/src/bin/unit_test_module.c +++ b/src/bin/unit_test_module.c @@ -693,13 +693,21 @@ int main(int argc, char *argv[]) fr_dict_t *dict = NULL; char const *receipt_file = NULL; - TALLOC_CTX *autofree = talloc_autofree_context(); - TALLOC_CTX *thread_ctx = talloc_new(autofree); + TALLOC_CTX *autofree; + TALLOC_CTX *thread_ctx; char *p; main_config_t *config; dl_module_loader_t *dl_modules = NULL; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + thread_ctx = talloc_new(autofree); + config = main_config_alloc(autofree); if (!config) { fr_perror("unit_test_module"); diff --git a/src/lib/util/thread_local.c b/src/lib/util/thread_local.c index ec0565ff03c..e29f202e4f0 100644 --- a/src/lib/util/thread_local.c +++ b/src/lib/util/thread_local.c @@ -59,6 +59,7 @@ typedef struct { _Thread_local fr_exit_handler_list_t *thread_local_atexit = NULL; static fr_exit_handler_list_t *global_atexit = NULL; static pthread_mutex_t global_atexit_mutex = PTHREAD_MUTEX_INITIALIZER; +static bool is_exiting; /** Call the exit handler @@ -128,9 +129,31 @@ static int _global_list_free(fr_exit_handler_list_t *ehl) */ static void _global_free(void) { + pthread_mutex_lock(&global_atexit_mutex); + fr_cond_assert_msg(!is_exiting, "Global free function called multiple times"); + is_exiting = true; + pthread_mutex_unlock(&global_atexit_mutex); + TALLOC_FREE(global_atexit); } +/** Setup the atexit handler, should be called at the start of a program's execution + * + */ +int fr_thread_local_atexit_setup(void) +{ + if (global_atexit) return 0; + + global_atexit = talloc_zero(NULL, fr_exit_handler_list_t); + if (unlikely(!global_atexit)) return -1; + + fr_dlist_talloc_init(&global_atexit->head, fr_exit_handler_entry_t, entry); + talloc_set_destructor(global_atexit, _global_list_free); + atexit(_global_free); /* Call all remaining destructors at process exit */ + + return 0; +} + /** Add a new destructor * * @return @@ -139,20 +162,17 @@ static void _global_free(void) */ int fr_thread_local_atexit(fr_thread_local_atexit_t func, void const *uctx) { + int ret = 0; + /* * Initialise the global list containing all the thread-local * dlist destructors. */ pthread_mutex_lock(&global_atexit_mutex); - if (!global_atexit) { - global_atexit = talloc_zero(NULL, fr_exit_handler_list_t); - if (unlikely(!global_atexit)) return -1; - - fr_dlist_talloc_init(&global_atexit->head, fr_exit_handler_entry_t, entry); - talloc_set_destructor(global_atexit, _global_list_free); - atexit(_global_free); /* Call all remaining destructors at process exit */ - } + fr_cond_assert_msg(!is_exiting, "New atexit handlers should not be allocated whilst exiting"); + if (!global_atexit) ret = fr_thread_local_atexit_setup(); pthread_mutex_unlock(&global_atexit_mutex); + if (ret < 0) return ret; /* * Initialise the thread local list, just for pthread_exit(). diff --git a/src/lib/util/thread_local.h b/src/lib/util/thread_local.h index cc6fc60050a..c3adab35134 100644 --- a/src/lib/util/thread_local.h +++ b/src/lib/util/thread_local.h @@ -38,6 +38,8 @@ extern "C" { typedef void(*fr_thread_local_atexit_t)(void *uctx); +int fr_thread_local_atexit_setup(void); + int fr_thread_local_atexit(fr_thread_local_atexit_t func, void const *uctx); /** Set a destructor for thread local storage to free the memory on thread exit diff --git a/src/modules/proto_control/radmin.c b/src/modules/proto_control/radmin.c index bf8dcc9e111..0439e26fa3f 100644 --- a/src/modules/proto_control/radmin.c +++ b/src/modules/proto_control/radmin.c @@ -843,7 +843,7 @@ int main(int argc, char **argv) char history_file[PATH_MAX]; #endif - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; char *commands[MAX_COMMANDS]; int num_commands = -1; @@ -853,6 +853,13 @@ int main(int argc, char **argv) char const *prompt = "radmin> "; char prompt_buffer[1024]; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + #ifndef NDEBUG if (fr_fault_setup(autofree, getenv("PANIC_ACTION"), argv[0]) < 0) { fr_perror("radmin"); diff --git a/src/modules/proto_dhcpv4/dhcpclient.c b/src/modules/proto_dhcpv4/dhcpclient.c index 976fb0c23b5..79209baa265 100644 --- a/src/modules/proto_dhcpv4/dhcpclient.c +++ b/src/modules/proto_dhcpv4/dhcpclient.c @@ -612,10 +612,17 @@ int main(int argc, char **argv) RADIUS_PACKET *packet = NULL; RADIUS_PACKET *reply = NULL; - TALLOC_CTX *autofree = talloc_autofree_context(); + TALLOC_CTX *autofree; int ret; + /* + * Must be called first, so the handler is called last + */ + fr_thread_local_atexit_setup(); + + autofree = talloc_autofree_context(); + fr_debug_lvl = 1; while ((c = getopt(argc, argv, "d:D:f:hr:t:vxi:")) != -1) switch(c) {