From: Arran Cudbard-Bell Date: Sat, 19 Feb 2022 04:50:01 +0000 (-0500) Subject: Explicitly cleanup "thread local" memory in single threaded mode X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=02e2091c592eab561f374b710b889625f0267676;p=thirdparty%2Ffreeradius-server.git Explicitly cleanup "thread local" memory in single threaded mode --- diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index f0e291e765e..7dcfd914d2d 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -1021,7 +1021,7 @@ int main(int argc, char *argv[]) fr_event_loop_exit(el, 1); } - main_loop_free(); /* Free the requests */ + main_loop_free(); /* * Send a TERM signal to all associated processes @@ -1051,6 +1051,16 @@ cleanup: */ (void) fr_schedule_destroy(&sc); + /* + * Ensure all thread local memory is cleaned up + * before we start cleaning up global resources. + * This is necessay for single threaded mode + * to ensure that thread local resources that + * depend on global resources are freed at the + * appropriate time. + */ + fr_atexit_thread_trigger_all(); + /* * Frees request specific logging resources which is OK * because all the requests will have been stopped. diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c index 0232aa4bfdc..f7aac270d50 100644 --- a/src/bin/unit_test_attribute.c +++ b/src/bin/unit_test_attribute.c @@ -3693,6 +3693,14 @@ int main(int argc, char *argv[]) * memory, so we get clean talloc reports. */ cleanup: + /* + * Ensure all thread local memory is cleaned up + * at the appropriate time. This emulates what's + * done with worker/network threads in the + * scheduler. + */ + fr_atexit_thread_trigger_all(); + #ifdef WITH_TLS fr_openssl_free(); #endif diff --git a/src/bin/unit_test_map.c b/src/bin/unit_test_map.c index 82df15ec0b4..1da27813455 100644 --- a/src/bin/unit_test_map.c +++ b/src/bin/unit_test_map.c @@ -272,6 +272,14 @@ int main(int argc, char *argv[]) if (ret < 0) ret = 1; /* internal to Unix process return code */ cleanup: + /* + * Ensure all thread local memory is cleaned up + * at the appropriate time. This emulates what's + * done with worker/network threads in the + * scheduler. + */ + fr_atexit_thread_trigger_all(); + /* * Try really hard to free any allocated * memory, so we get clean talloc reports. diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c index 6d6dafb921f..e8e3c699525 100644 --- a/src/bin/unit_test_module.c +++ b/src/bin/unit_test_module.c @@ -1004,6 +1004,14 @@ cleanup: */ talloc_free(el); + /* + * Ensure all thread local memory is cleaned up + * at the appropriate time. This emulates what's + * done with worker/network threads in the + * scheduler. + */ + fr_atexit_thread_trigger_all(); + /* * Free request specific logging infrastructure */ diff --git a/src/lib/tls/log.c b/src/lib/tls/log.c index e25d765eeec..d26be319a3b 100644 --- a/src/lib/tls/log.c +++ b/src/lib/tls/log.c @@ -31,6 +31,7 @@ USES_APPLE_DEPRECATED_API /* OpenSSL API has been deprecated by Apple */ #include #include +#include #include "log.h" #include "utils.h" @@ -78,6 +79,16 @@ static BIO_METHOD *tls_request_log_meth; */ static BIO_METHOD *tls_global_log_meth; +/** Counter for users of the request log bio + * + */ +static _Atomic(uint32_t) tls_request_log_ref; + +/** Counter for users of the global log bio + * + */ +static _Atomic(uint32_t) tls_global_log_ref; + /** Thread local request log BIO */ static _Thread_local fr_tls_log_bio_t *request_log_bio; @@ -541,6 +552,15 @@ void tls_log_clear(void) while (ERR_get_error() != 0); } +/** Increment the bio meth reference counter + * + */ +static int tls_log_request_bio_create_cb(UNUSED BIO *bio) +{ + atomic_fetch_add(&tls_request_log_ref, 1); + return 1; +} + /** Converts BIO_write() calls to request log calls * * This callback is used to glue the output of OpenSSL functions into request log calls. @@ -615,6 +635,24 @@ static int tls_log_request_bio_puts_cb(BIO *bio, char const *in) return tls_log_request_bio_write_cb(bio, in, strlen(in)); } +/** Decrement the bio meth reference counter + * + */ +static int tls_log_request_bio_free_cb(UNUSED BIO *bio) +{ + atomic_fetch_sub(&tls_request_log_ref, 1); + return 1; +} + +/** Increment the bio meth reference counter + * + */ +static int tls_log_global_bio_create_cb(UNUSED BIO *bio) +{ + atomic_fetch_add(&tls_global_log_ref, 1); + return 1; +} + /** Converts BIO_write() calls to global log calls * * This callback is used to glue the output of OpenSSL functions into global log calls. @@ -679,6 +717,15 @@ static int tls_log_global_bio_puts_cb(BIO *bio, char const *in) return tls_log_global_bio_write_cb(bio, in, strlen(in)); } +/** Decrement the bio meth reference counter + * + */ +static int tls_log_global_bio_free_cb(UNUSED BIO *bio) +{ + atomic_fetch_sub(&tls_global_log_ref, 1); + return 1; +} + /** Frees a logging bio and its underlying OpenSSL BIO * * */ @@ -802,8 +849,10 @@ int fr_tls_log_init(void) tls_request_log_meth = BIO_meth_new(BIO_get_new_index() | BIO_TYPE_SOURCE_SINK, "fr_tls_request_log"); if (unlikely(!tls_request_log_meth)) return -1; + BIO_meth_set_create(tls_request_log_meth, tls_log_request_bio_create_cb); BIO_meth_set_write(tls_request_log_meth, tls_log_request_bio_write_cb); BIO_meth_set_puts(tls_request_log_meth, tls_log_request_bio_puts_cb); + BIO_meth_set_destroy(tls_request_log_meth, tls_log_request_bio_free_cb); tls_global_log_meth = BIO_meth_new(BIO_get_new_index() | BIO_TYPE_SOURCE_SINK, "fr_tls_global_log"); if (unlikely(!tls_global_log_meth)) { @@ -812,8 +861,10 @@ int fr_tls_log_init(void) return -1; } + BIO_meth_set_create(tls_global_log_meth, tls_log_global_bio_create_cb); BIO_meth_set_write(tls_global_log_meth, tls_log_global_bio_write_cb); BIO_meth_set_puts(tls_global_log_meth, tls_log_global_bio_puts_cb); + BIO_meth_set_destroy(tls_global_log_meth, tls_log_global_bio_free_cb); return 0; } @@ -823,6 +874,14 @@ int fr_tls_log_init(void) */ void fr_tls_log_free(void) { + /* + * These must be freed first else + * we get crashes in the OpenSSL + * code when we try to free them. + */ + fr_assert_msg(atomic_load(&tls_request_log_ref) == 0, "request log BIO refs remaining %u", atomic_load(&tls_request_log_ref)); + fr_assert_msg(atomic_load(&tls_global_log_ref) == 0, "global log BIO refs remaining %u", atomic_load(&tls_global_log_ref)); + if (tls_request_log_meth) { BIO_meth_free(tls_request_log_meth); tls_request_log_meth = NULL; diff --git a/src/lib/util/atexit.c b/src/lib/util/atexit.c index 689605bcf43..ea8acd8939e 100644 --- a/src/lib/util/atexit.c +++ b/src/lib/util/atexit.c @@ -469,3 +469,42 @@ do_threads: return count; } + +/** Cause all thread local free triggers to fire + * + * This is necessary when we're running in single threaded mode + * to ensure all "thread-local" memory (which isn't actually thread local) + * is cleaned up. + * + * One example is the OpenSSL log BIOs which must be cleaned up + * before fr_openssl_free is called. + */ +unsigned int fr_atexit_thread_trigger_all(void) +{ + fr_atexit_entry_t *e = NULL, *ee; + fr_atexit_list_t *list; + unsigned int count = 0; + + /* + * Iterate over the list of thread local + * destructor lists running the + * destructors. + */ + while ((e = fr_dlist_next(&fr_atexit_threads->head, e))) { + if (!e->func) continue; /* thread already joined */ + + list = talloc_get_type_abort(e->uctx, fr_atexit_list_t); + ee = NULL; + while ((ee = fr_dlist_next(&list->head, ee))) { + ATEXIT_DEBUG("%s - Thread %u triggering %p/%p func=%p, uctx=%p (alloced %s:%u)", + __FUNCTION__, + (unsigned int)pthread_self(), + list, ee, ee->func, ee->uctx, ee->file, ee->line); + + count++; + ee = fr_dlist_talloc_free_item(&list->head, ee); + } + } + + return count; +} diff --git a/src/lib/util/atexit.h b/src/lib/util/atexit.h index 2f329c67f76..ae013197074 100644 --- a/src/lib/util/atexit.h +++ b/src/lib/util/atexit.h @@ -123,6 +123,8 @@ void fr_atexit_global_disarm_all(void); unsigned int fr_atexit_trigger(bool uctx_scope, fr_atexit_t func, void const *uctx); +unsigned int fr_atexit_thread_trigger_all(void); + #ifdef __cplusplus } #endif diff --git a/src/lib/util/talloc.c b/src/lib/util/talloc.c index 6dba9ed25d6..947a97d1f80 100644 --- a/src/lib/util/talloc.c +++ b/src/lib/util/talloc.c @@ -28,7 +28,7 @@ RCSID("$Id$") #include #include - +static TALLOC_CTX *global_ctx; static _Thread_local TALLOC_CTX *thread_local_ctx; /** A wrapper that can be passed to tree or hash alloc functions that take a #fr_free_t @@ -761,10 +761,10 @@ void **talloc_array_null_strip(void **array) return new; } -/** Callback to free the autofree ctx on thread exit +/** Callback to free the autofree ctx on global exit * */ -static void _autofree_on_thread_exit(void *af) +static void _autofree_on_exit(void *af) { talloc_set_destructor(af, NULL); talloc_free(af); @@ -773,9 +773,33 @@ static void _autofree_on_thread_exit(void *af) /** Ensures in the autofree ctx is manually freed, things don't explode atexit * */ -static int _autofree_destructor(TALLOC_CTX *af) +static int _autofree_global_destructor(TALLOC_CTX *af) +{ + return fr_atexit_global_disarm(true, _autofree_on_exit, af); +} + +TALLOC_CTX *talloc_autofree_context_global(void) +{ + TALLOC_CTX *af = global_ctx; + + if (!af) { + af = talloc_init_const("global_autofree_context"); + talloc_set_destructor(af, _autofree_global_destructor); + if (unlikely(!af)) return NULL; + + fr_atexit_global(_autofree_on_exit, af); + global_ctx = af; + } + + return af; +} + +/** Ensures in the autofree ctx is manually freed, things don't explode atexit + * + */ +static int _autofree_thread_local_destructor(TALLOC_CTX *af) { - return fr_atexit_thread_local_disarm(true, _autofree_on_thread_exit, af); + return fr_atexit_thread_local_disarm(true, _autofree_on_exit, af); } /** Get a thread-safe autofreed ctx that will be freed when the thread or process exits @@ -791,15 +815,16 @@ TALLOC_CTX *talloc_autofree_context_thread_local(void) if (!af) { af = talloc_init_const("thread_local_autofree_context"); - talloc_set_destructor(af, _autofree_destructor); + talloc_set_destructor(af, _autofree_thread_local_destructor); if (unlikely(!af)) return NULL; - fr_atexit_thread_local(thread_local_ctx, _autofree_on_thread_exit, af); + fr_atexit_thread_local(thread_local_ctx, _autofree_on_exit, af); } return af; } + struct talloc_child_ctx_s { struct talloc_child_ctx_s *next; }; diff --git a/src/lib/util/talloc.h b/src/lib/util/talloc.h index 65691c20333..48c4855b305 100644 --- a/src/lib/util/talloc.h +++ b/src/lib/util/talloc.h @@ -46,7 +46,7 @@ DIAG_ON(documentation) #undef talloc_autofree_context /** The original function is deprecated, so replace it with our version */ -#define talloc_autofree_context talloc_autofree_context_thread_local +#define talloc_autofree_context talloc_autofree_context_global /** Iterate over a talloced array of elements * @@ -263,6 +263,7 @@ static inline void *_talloc_list_get_type_abort(void *head, size_t offset, char # define talloc_get_type_abort_const talloc_get_type_abort #endif +TALLOC_CTX *talloc_autofree_context_global(void); TALLOC_CTX *talloc_autofree_context_thread_local(void); typedef struct talloc_child_ctx_s TALLOC_CHILD_CTX;