From 01731d4b1ba409ca05565b69e35c02fd65adbdd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 1/8] Add and use ISC_THREAD_LOCAL macro The new ISC_THREAD_LOCAL macro unifies usage of platform dependent Thread Local Storage definition thread_local vs __thread vs __declspec(thread) to a single macro. The commit also unifies the required level of support for TLS as for some parts of the code it was mandatory and for some parts of the code it wasn't. --- configure | 6 +-- configure.ac | 4 +- lib/isc/hp.c | 16 +------- lib/isc/include/isc/platform.h.in | 8 ---- lib/isc/netmgr/netmgr-int.h | 1 - lib/isc/netmgr/netmgr.c | 15 +------ lib/isc/pthreads/include/isc/thread.h | 17 ++++++++ lib/isc/random.c | 16 +------- lib/isc/win32/include/isc/thread.h | 6 +++ lib/isc/xoshiro128starstar.c | 57 ++------------------------- 10 files changed, 35 insertions(+), 111 deletions(-) diff --git a/configure b/configure index 0d6df8eea0..7d1629af0e 100755 --- a/configure +++ b/configure @@ -13616,8 +13616,7 @@ $as_echo "#define HAVE_TLS 1" >>confdefs.h else - { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 -$as_echo "no" >&6; } + as_fn_error $? "Thread Local Storage support required, update your toolchain to build BIND 9" "$LINENO" 5 fi rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext @@ -13655,8 +13654,7 @@ $as_echo "#define HAVE_TLS 1" >>confdefs.h else - { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 -$as_echo "no" >&6; } + as_fn_error $? "Thread Local Storage support required, update your toolchain to build BIND 9" "$LINENO" 5 fi rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext diff --git a/configure.ac b/configure.ac index 623cb277d1..25b0ce8959 100644 --- a/configure.ac +++ b/configure.ac @@ -389,7 +389,7 @@ AC_CHECK_HEADERS([threads.h], AC_DEFINE([HAVE_THREAD_LOCAL],[1],[Define if thread_local keyword is available]) AC_DEFINE([HAVE_TLS],[1],[Define if Thread-Local Storage is available]) ],[ - AC_MSG_RESULT([no]) + AC_MSG_ERROR([Thread Local Storage support required, update your toolchain to build BIND 9]) ]) ],[ AC_MSG_CHECKING([for Thread-Local Storage using __thread]) @@ -405,7 +405,7 @@ AC_CHECK_HEADERS([threads.h], AC_DEFINE([HAVE___THREAD],[1],[Define if __thread keyword is available]) AC_DEFINE([HAVE_TLS],[1],[Define if Thread-Local Storage is available]) ],[ - AC_MSG_RESULT([no]) + AC_MSG_ERROR([Thread Local Storage support required, update your toolchain to build BIND 9]) ]) ]) diff --git a/lib/isc/hp.c b/lib/isc/hp.c index 746d13fc0a..4bdd23c284 100644 --- a/lib/isc/hp.c +++ b/lib/isc/hp.c @@ -51,6 +51,7 @@ #include #include #include +#include #define HP_MAX_THREADS 128 #define HP_MAX_HPS 4 /* This is named 'K' in the HP paper */ @@ -64,20 +65,7 @@ static atomic_int_fast32_t tid_v_base = ATOMIC_VAR_INIT(0); -#if defined(HAVE_TLS) -#if defined(HAVE_THREAD_LOCAL) -#include -static thread_local int tid_v = TID_UNKNOWN; -#elif defined(HAVE___THREAD) -static __thread int tid_v = TID_UNKNOWN; -#elif defined(HAVE___DECLSPEC_THREAD) -static __declspec( thread ) int tid_v = TID_UNKNOWN; -#else /* if defined(HAVE_THREAD_LOCAL) */ -#error "Unknown method for defining a TLS variable!" -#endif /* if defined(HAVE_THREAD_LOCAL) */ -#else /* if defined(HAVE_TLS) */ -#error "Thread-local storage support is required!" -#endif /* if defined(HAVE_TLS) */ +ISC_THREAD_LOCAL int tid_v = TID_UNKNOWN; typedef struct retirelist { int size; diff --git a/lib/isc/include/isc/platform.h.in b/lib/isc/include/isc/platform.h.in index 0a16b88739..68c0103834 100644 --- a/lib/isc/include/isc/platform.h.in +++ b/lib/isc/include/isc/platform.h.in @@ -18,14 +18,6 @@ ***** Platform-dependent defines. *****/ -/*** - *** Thread-Local Storage - ***/ - -#ifdef HAVE___THREAD -#define thread_local __thread -#endif - /*** *** Default strerror_r buffer size ***/ diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index b4191ff6ef..3acacc756b 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -31,7 +31,6 @@ #include #define ISC_NETMGR_TID_UNKNOWN -1 -#define ISC_NETMGR_TID_NOTLS -2 /* * Single network event loop worker. diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index e8d330f008..902fe2d1cc 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -40,20 +40,7 @@ * request using async_cb. */ -#if defined(HAVE_TLS) -#if defined(HAVE_THREAD_LOCAL) -#include -static thread_local int isc__nm_tid_v = ISC_NETMGR_TID_UNKNOWN; -#elif defined(HAVE___THREAD) -static __thread int isc__nm_tid_v = ISC_NETMGR_TID_UNKNOWN; -#elif defined(HAVE___DECLSPEC_THREAD) -static __declspec( thread ) int isc__nm_tid_v = ISC_NETMGR_TID_UNKNOWN; -#else /* if defined(HAVE_THREAD_LOCAL) */ -#error "Unknown method for defining a TLS variable!" -#endif /* if defined(HAVE_THREAD_LOCAL) */ -#else /* if defined(HAVE_TLS) */ -static int isc__nm_tid_v = ISC_NETMGR_TID_NOTLS; -#endif /* if defined(HAVE_TLS) */ +ISC_THREAD_LOCAL int isc__nm_tid_v = ISC_NETMGR_TID_UNKNOWN; static void nmsocket_maybe_destroy(isc_nmsocket_t *sock); diff --git a/lib/isc/pthreads/include/isc/thread.h b/lib/isc/pthreads/include/isc/thread.h index 63194d3b04..ac50cce0fd 100644 --- a/lib/isc/pthreads/include/isc/thread.h +++ b/lib/isc/pthreads/include/isc/thread.h @@ -58,6 +58,23 @@ isc_thread_setaffinity(int cpu); #define isc_thread_key_setspecific pthread_setspecific #define isc_thread_key_delete pthread_key_delete +/*** + *** Thread-Local Storage + ***/ + +#if defined(HAVE_TLS) +#if defined(HAVE_THREAD_LOCAL) +#include +#define ISC_THREAD_LOCAL static thread_local +#elif defined(HAVE___THREAD) +#define ISC_THREAD_LOCAL static __thread +#else /* if defined(HAVE_THREAD_LOCAL) */ +#error "Unknown method for defining a TLS variable!" +#endif /* if defined(HAVE_THREAD_LOCAL) */ +#else /* if defined(HAVE_TLS) */ +#error "Thread-local storage support is required!" +#endif /* if defined(HAVE_TLS) */ + ISC_LANG_ENDDECLS #endif /* ISC_THREAD_H */ diff --git a/lib/isc/random.c b/lib/isc/random.c index 79fbc7c7aa..681478f6b0 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -60,20 +61,7 @@ */ #include "xoshiro128starstar.c" -#if defined(HAVE_TLS) -#if defined(HAVE_THREAD_LOCAL) -#include -static thread_local isc_once_t isc_random_once = ISC_ONCE_INIT; -#elif defined(HAVE___THREAD) -static __thread isc_once_t isc_random_once = ISC_ONCE_INIT; -#elif defined(HAVE___DECLSPEC_THREAD) -static __declspec( thread ) isc_once_t isc_random_once = ISC_ONCE_INIT; -#else -#error "Unknown method for defining a TLS variable!" -#endif -#else -static isc_once_t isc_random_once = ISC_ONCE_INIT; -#endif +ISC_THREAD_LOCAL isc_once_t isc_random_once = ISC_ONCE_INIT; static void isc_random_initialize(void) { diff --git a/lib/isc/win32/include/isc/thread.h b/lib/isc/win32/include/isc/thread.h index f091d0623c..05225f9016 100644 --- a/lib/isc/win32/include/isc/thread.h +++ b/lib/isc/win32/include/isc/thread.h @@ -96,6 +96,12 @@ isc_thread_key_setspecific(isc_thread_key_t key, void *value); #define isc_thread_yield() Sleep(0) +#if HAVE___DECLSPEC_THREAD +#define ISC_THREAD_LOCAL static __declspec( thread ) +#else +#error "Thread-local storage support is required!" +#endif + ISC_LANG_ENDDECLS #endif /* ISC_THREAD_H */ diff --git a/lib/isc/xoshiro128starstar.c b/lib/isc/xoshiro128starstar.c index abf1ea8455..589ed54e60 100644 --- a/lib/isc/xoshiro128starstar.c +++ b/lib/isc/xoshiro128starstar.c @@ -22,6 +22,8 @@ #include +#include + /* * This is xoshiro128** 1.0, our 32-bit all-purpose, rock-solid generator. * It has excellent (sub-ns) speed, a state size (128 bits) that is large @@ -32,56 +34,7 @@ * * The state must be seeded so that it is not everywhere zero. */ -#if defined(HAVE_TLS) -#define _LOCK() {}; -#define _UNLOCK() {}; - -#if defined(HAVE_THREAD_LOCAL) -#include -static thread_local uint32_t seed[4]; -#elif defined(HAVE___THREAD) -static __thread uint32_t seed[4]; -#elif defined(HAVE___DECLSPEC_THREAD) -static __declspec( thread ) uint32_t seed[4]; -#else -#error "Unknown method for defining a TLS variable!" -#endif - -#else -#if defined(_WIN32) || defined(_WIN64) -#include -static volatile void *_mutex = NULL; - -/* - * Initialize the mutex on the first lock attempt. On collision, each thread - * will attempt to allocate a mutex and compare-and-swap it into place as the - * global mutex. On failure to swap in the global mutex, the mutex is closed. - */ -#define _LOCK() \ - do { \ - if (!_mutex) { \ - void *p = CreateMutex(NULL, FALSE, NULL); \ - if (InterlockedCompareExchangePointer \ - ((void **)&_mutex, (void *)p, NULL)) { \ - CloseHandle(p); \ - } \ - } \ - WaitForSingleObject(_mutex, INFINITE); \ - } while (0) - -#define _UNLOCK() ReleaseMutex(_mutex) - -#else /* defined(_WIN32) || defined(_WIN64) */ - -#include -static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER; -#define _LOCK() RUNTIME_CHECK(pthread_mutex_lock(&_mutex)==0) -#define _UNLOCK() RUNTIME_CHECK(pthread_mutex_unlock(&_mutex)==0) -#endif /* defined(_WIN32) || defined(_WIN64) */ - -static uint32_t seed[4]; - -#endif /* defined(HAVE_TLS) */ +ISC_THREAD_LOCAL uint32_t seed[4] = { 0 }; static inline uint32_t rotl(const uint32_t x, int k) { return (x << k) | (x >> (32 - k)); @@ -91,8 +44,6 @@ static inline uint32_t next(void) { uint32_t result_starstar, t; - _LOCK(); - result_starstar = rotl(seed[0] * 5, 7) * 9; t = seed[1] << 9; @@ -105,7 +56,5 @@ next(void) { seed[3] = rotl(seed[3], 11); - _UNLOCK(); - return (result_starstar); } From 1a66aabd22efc08c8d22e43377dd5d6f4f594dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 2/8] Refactor the dns_name API to use ISC_THREAD_LOCAL Previously, the dns_name API used isc_thread_key API for TLS, which is fairly complicated and requires initialization of memory contexts, etc. This part of code was refactored to use a ISC_THREAD_LOCAL pointer which greatly simplifies the whole code related to storing TLS variables. --- bin/check/named-checkconf.c | 2 - bin/check/named-checkzone.c | 4 +- bin/dig/dighost.c | 1 - bin/dnssec/dnssec-dsfromkey.c | 1 - bin/dnssec/dnssec-importkey.c | 1 - bin/dnssec/dnssec-keyfromlabel.c | 1 - bin/dnssec/dnssec-keygen.c | 1 - bin/dnssec/dnssec-signzone.c | 1 - bin/dnssec/dnssec-verify.c | 1 - bin/named/main.c | 2 - bin/nsupdate/nsupdate.c | 3 - bin/rndc/rndc.c | 2 - bin/tests/system/rsabigexponent/bigkey.c | 1 - lib/dns/include/dns/name.h | 18 +--- lib/dns/name.c | 120 +++-------------------- lib/dns/win32/libdns.def.in | 1 - 16 files changed, 20 insertions(+), 140 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index c8b12911bb..0e5bde922f 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -709,8 +709,6 @@ main(int argc, char **argv) { cfg_parser_destroy(&parser); - dns_name_destroy(); - isc_log_destroy(&logc); isc_mem_destroy(&mctx); diff --git a/bin/check/named-checkzone.c b/bin/check/named-checkzone.c index de6bb94b8e..67613de41d 100644 --- a/bin/check/named-checkzone.c +++ b/bin/check/named-checkzone.c @@ -85,9 +85,9 @@ usage(void) { static void destroy(void) { - if (zone != NULL) + if (zone != NULL) { dns_zone_detach(&zone); - dns_name_destroy(); + } } /*% main processing routine */ diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index bc712a1ad0..b9bc11603b 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -4252,7 +4252,6 @@ destroy_libs(void) { result = dns_name_settotextfilter(NULL); check_result(result, "dns_name_settotextfilter"); #endif /* HAVE_LIBIDN2 */ - dns_name_destroy(); if (commctx != NULL) { debug("freeing commctx"); diff --git a/bin/dnssec/dnssec-dsfromkey.c b/bin/dnssec/dnssec-dsfromkey.c index 4923fe6e8a..95ba8e9e7e 100644 --- a/bin/dnssec/dnssec-dsfromkey.c +++ b/bin/dnssec/dnssec-dsfromkey.c @@ -517,7 +517,6 @@ main(int argc, char **argv) { } cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) { isc_mem_stats(mctx, stdout); } diff --git a/bin/dnssec/dnssec-importkey.c b/bin/dnssec/dnssec-importkey.c index 964efcad0e..4a4684d741 100644 --- a/bin/dnssec/dnssec-importkey.c +++ b/bin/dnssec/dnssec-importkey.c @@ -439,7 +439,6 @@ main(int argc, char **argv) { dns_rdataset_disassociate(&rdataset); cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); isc_mem_destroy(&mctx); diff --git a/bin/dnssec/dnssec-keyfromlabel.c b/bin/dnssec/dnssec-keyfromlabel.c index e9dfe36f4f..4bc653f3a5 100644 --- a/bin/dnssec/dnssec-keyfromlabel.c +++ b/bin/dnssec/dnssec-keyfromlabel.c @@ -694,7 +694,6 @@ main(int argc, char **argv) { cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); isc_mem_free(mctx, label); diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 40cffe2168..220cbe7a00 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -1222,7 +1222,6 @@ main(int argc, char **argv) { cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); isc_mem_destroy(&mctx); diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 7682a178e9..8008ed7712 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3922,7 +3922,6 @@ main(int argc, char *argv[]) { cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); isc_mem_destroy(&mctx); diff --git a/bin/dnssec/dnssec-verify.c b/bin/dnssec/dnssec-verify.c index 483577ef4f..29894c9e81 100644 --- a/bin/dnssec/dnssec-verify.c +++ b/bin/dnssec/dnssec-verify.c @@ -335,7 +335,6 @@ main(int argc, char *argv[]) { cleanup_logging(&log); dst_lib_destroy(); - dns_name_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); isc_mem_destroy(&mctx); diff --git a/bin/named/main.c b/bin/named/main.c index 10e4f536a0..f1c14c511d 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -1329,8 +1329,6 @@ cleanup(void) { dlz_dlopen_clear(); #endif - dns_name_destroy(); - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_MAIN, ISC_LOG_NOTICE, "exiting"); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index a272b14a0f..4b4ce5cfad 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -3210,9 +3210,6 @@ cleanup(void) { ddebug("Shutting down timer manager"); isc_timermgr_destroy(&timermgr); - ddebug("Destroying name state"); - dns_name_destroy(); - ddebug("Removing log context"); isc_log_destroy(&glctx); diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index d9f32a4507..cc91793859 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -1022,8 +1022,6 @@ main(int argc, char **argv) { isc_mem_put(rndc_mctx, args, argslen); isccc_ccmsg_invalidate(&ccmsg); - dns_name_destroy(); - isc_buffer_free(&databuf); if (show_final_mem) diff --git a/bin/tests/system/rsabigexponent/bigkey.c b/bin/tests/system/rsabigexponent/bigkey.c index d01d264832..a1c4a01c4f 100644 --- a/bin/tests/system/rsabigexponent/bigkey.c +++ b/bin/tests/system/rsabigexponent/bigkey.c @@ -145,7 +145,6 @@ main(int argc, char **argv) { isc_log_setcontext(NULL); dns_log_setcontext(NULL); dst_lib_destroy(); - dns_name_destroy(); isc_mem_destroy(&mctx); return (0); #else /* !USE_PKCS11 */ diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h index 6b50492491..2c051265fc 100644 --- a/lib/dns/include/dns/name.h +++ b/lib/dns/include/dns/name.h @@ -197,8 +197,8 @@ LIBDNS_EXTERNAL_DATA extern const dns_name_t *dns_wildcardname; * 'target' is the buffer to be converted. The region to be converted * is from 'buffer'->base + 'used_org' to the end of the used region. */ -typedef isc_result_t (*dns_name_totextfilter_t)(isc_buffer_t *target, - unsigned int used_org); +typedef isc_result_t (dns_name_totextfilter_t)(isc_buffer_t *target, + unsigned int used_org); /*** *** Initialization @@ -1217,7 +1217,7 @@ dns_name_fromstring2(dns_name_t *target, const char *src, */ isc_result_t -dns_name_settotextfilter(dns_name_totextfilter_t proc); +dns_name_settotextfilter(dns_name_totextfilter_t *proc); /*%< * Set / clear a thread specific function 'proc' to be called at the * end of dns_name_totext(). @@ -1296,18 +1296,6 @@ dns_name_internalwildcard(const dns_name_t *name); * \li 'name' to be valid. */ -void -dns_name_destroy(void); -/*%< - * Cleanup dns_name_settotextfilter() / dns_name_totext() state. - * - * This should be called as part of the final cleanup process. - * - * Note: dns_name_settotextfilter(NULL); should be called for all - * threads which have called dns_name_settotextfilter() with a - * non-NULL argument prior to calling dns_name_destroy(); - */ - bool dns_name_isdnssd(const dns_name_t *owner); /*%< diff --git a/lib/dns/name.c b/lib/dns/name.c index 3bb2e4fcf8..82b546429e 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -163,11 +163,7 @@ LIBDNS_EXTERNAL_DATA const dns_name_t *dns_wildcardname = &wild; /* * dns_name_t to text post-conversion procedure. */ -static int thread_key_initialized = 0; -static isc_mutex_t thread_key_mutex; -static isc_mem_t *thread_key_mctx = NULL; -static isc_thread_key_t totext_filter_proc_key; -static isc_once_t once = ISC_ONCE_INIT; +ISC_THREAD_LOCAL dns_name_totextfilter_t *totext_filter_proc = NULL; static void set_offsets(const dns_name_t *name, unsigned char *offsets, @@ -1272,52 +1268,6 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source, return (ISC_R_SUCCESS); } -static void -free_specific(void *arg) { - dns_name_totextfilter_t *mem = arg; - isc_mem_put(thread_key_mctx, mem, sizeof(*mem)); - /* Stop use being called again. */ - (void)isc_thread_key_setspecific(totext_filter_proc_key, NULL); -} - -static void -thread_key_mutex_init(void) { - isc_mutex_init(&thread_key_mutex); -} - -static isc_result_t -totext_filter_proc_key_init(void) { - isc_result_t result; - - /* - * We need the call to isc_once_do() to support profiled mutex - * otherwise thread_key_mutex could be initialized at compile time. - */ - result = isc_once_do(&once, thread_key_mutex_init); - if (result != ISC_R_SUCCESS) - return (result); - - if (!thread_key_initialized) { - LOCK(&thread_key_mutex); - if (thread_key_mctx == NULL) { - isc_mem_create(&thread_key_mctx); - } - isc_mem_setname(thread_key_mctx, "threadkey", NULL); - isc_mem_setdestroycheck(thread_key_mctx, false); - - if (!thread_key_initialized && - isc_thread_key_create(&totext_filter_proc_key, - free_specific) != 0) { - result = ISC_R_FAILURE; - isc_mem_detach(&thread_key_mctx); - } else { - thread_key_initialized = 1; - } - UNLOCK(&thread_key_mutex); - } - return (result); -} - isc_result_t dns_name_totext(const dns_name_t *name, bool omit_final_dot, isc_buffer_t *target) @@ -1346,9 +1296,6 @@ dns_name_totext2(const dns_name_t *name, unsigned int options, unsigned int labels; bool saw_root = false; unsigned int oused; - dns_name_totextfilter_t *mem; - dns_name_totextfilter_t totext_filter_proc = NULL; - isc_result_t result; bool omit_final_dot = ((options & DNS_NAME_OMITFINALDOT) != 0); /* @@ -1360,9 +1307,6 @@ dns_name_totext2(const dns_name_t *name, unsigned int options, oused = target->used; - result = totext_filter_proc_key_init(); - if (result != ISC_R_SUCCESS) - return (result); ndata = name->ndata; nlen = name->length; labels = name->labels; @@ -1502,11 +1446,9 @@ dns_name_totext2(const dns_name_t *name, unsigned int options, } isc_buffer_add(target, tlen - trem); - mem = isc_thread_key_getspecific(totext_filter_proc_key); - if (mem != NULL) - totext_filter_proc = *mem; - if (totext_filter_proc != NULL) - return ((*totext_filter_proc)(target, oused)); + if (totext_filter_proc != NULL) { + return ((totext_filter_proc)(target, oused)); + } return (ISC_R_SUCCESS); } @@ -2320,40 +2262,23 @@ dns_name_print(const dns_name_t *name, FILE *stream) { } isc_result_t -dns_name_settotextfilter(dns_name_totextfilter_t proc) { - isc_result_t result; - dns_name_totextfilter_t *mem; - int res; - - result = totext_filter_proc_key_init(); - if (result != ISC_R_SUCCESS) - return (result); - +dns_name_settotextfilter(dns_name_totextfilter_t *proc) { /* * If we already have been here set / clear as appropriate. - * Otherwise allocate memory. */ - mem = isc_thread_key_getspecific(totext_filter_proc_key); - if (mem != NULL && proc != NULL) { - *mem = proc; + if (totext_filter_proc != NULL && proc != NULL) { + if (totext_filter_proc == proc) { + return (ISC_R_SUCCESS); + } + } + if (proc == NULL && totext_filter_proc != NULL) { + totext_filter_proc = NULL; return (ISC_R_SUCCESS); } - if (proc == NULL) { - if (mem != NULL) - isc_mem_put(thread_key_mctx, mem, sizeof(*mem)); - res = isc_thread_key_setspecific(totext_filter_proc_key, NULL); - if (res != 0) - result = ISC_R_UNEXPECTED; - return (result); - } - mem = isc_mem_get(thread_key_mctx, sizeof(*mem)); - *mem = proc; - if (isc_thread_key_setspecific(totext_filter_proc_key, mem) != 0) { - isc_mem_put(thread_key_mctx, mem, sizeof(*mem)); - result = ISC_R_UNEXPECTED; - } - return (result); + totext_filter_proc = proc; + + return (ISC_R_SUCCESS); } void @@ -2511,21 +2436,6 @@ dns_name_copynf(const dns_name_t *source, dns_name_t *dest) RUNTIME_CHECK(name_copy(source, dest, dest->buffer) == ISC_R_SUCCESS); } -void -dns_name_destroy(void) { - RUNTIME_CHECK(isc_once_do(&once, thread_key_mutex_init) - == ISC_R_SUCCESS); - - LOCK(&thread_key_mutex); - if (thread_key_initialized) { - isc_mem_detach(&thread_key_mctx); - isc_thread_key_delete(totext_filter_proc_key); - thread_key_initialized = 0; - } - UNLOCK(&thread_key_mutex); - -} - /* * Service Discovery Prefixes RFC 6763. */ diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index b1a02a0f68..2eef22497c 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -584,7 +584,6 @@ dns_name_concatenate dns_name_copy dns_name_copynf dns_name_countlabels -dns_name_destroy dns_name_digest dns_name_downcase dns_name_dup From 4a3d589403dc2c8fc7dbdfe9ef6cf5ff2f2cb5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 3/8] Refactor the dns_dt API to use ISC_THREAD_LOCAL Previously, the dns_dt API used isc_thread_key API for TLS, which is fairly complicated and requires initialization of memory contexts, etc. This part of code was refactored to use a ISC_THREAD_LOCAL pointer which greatly simplifies the whole code related to storing TLS variables. --- bin/named/server.c | 3 - lib/dns/dnstap.c | 121 ++++++++--------------------------- lib/dns/include/dns/dnstap.h | 7 -- lib/dns/tests/dnstap_test.c | 3 - lib/dns/win32/libdns.def.in | 1 - 5 files changed, 28 insertions(+), 107 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 9321bc1dad..400bd15b46 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9742,9 +9742,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { dns_tsigkey_detach(&named_g_sessionkey); dns_name_free(&named_g_sessionkeyname, server->mctx); } -#ifdef HAVE_DNSTAP - dns_dt_shutdown(); -#endif #if defined(HAVE_GEOIP2) named_geoip_shutdown(); dns_geoip_shutdown(); diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c index 4e7d8e7b48..1f66035ce6 100644 --- a/lib/dns/dnstap.c +++ b/lib/dns/dnstap.c @@ -127,59 +127,16 @@ struct dns_dtenv { goto cleanup; \ } while (0) -static isc_mutex_t dt_mutex; -static bool dt_initialized = false; -static isc_thread_key_t dt_key; -static isc_once_t mutex_once = ISC_ONCE_INIT; -static isc_mem_t *dt_mctx = NULL; -/* - * Change under task exclusive. - */ -static unsigned int generation; +typedef struct ioq { + unsigned int generation; + struct fstrm_iothr_queue *ioq; +} dt__ioq_t; -static void -mutex_init(void) { - isc_mutex_init(&dt_mutex); -} -static void -dtfree(void *arg) { - free(arg); - isc_thread_key_setspecific(dt_key, NULL); -} +ISC_THREAD_LOCAL dt__ioq_t dt_ioq = { 0 }; -static isc_result_t -dt_init(void) { - isc_result_t result; - - result = isc_once_do(&mutex_once, mutex_init); - if (result != ISC_R_SUCCESS) - return (result); - - if (dt_initialized) - return (ISC_R_SUCCESS); - - LOCK(&dt_mutex); - if (!dt_initialized) { - int ret; - - if (dt_mctx == NULL) { - isc_mem_create(&dt_mctx); - } - isc_mem_setname(dt_mctx, "dt", NULL); - isc_mem_setdestroycheck(dt_mctx, false); - - ret = isc_thread_key_create(&dt_key, dtfree); - if (ret == 0) - dt_initialized = true; - else - result = ISC_R_FAILURE; - } - UNLOCK(&dt_mutex); - - return (result); -} +static atomic_uint_fast32_t global_generation; isc_result_t dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path, @@ -202,7 +159,7 @@ dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path, DNS_LOGMODULE_DNSTAP, ISC_LOG_INFO, "opening dnstap destination '%s'", path); - generation++; + atomic_fetch_add_release(&global_generation, 1); env = isc_mem_get(mctx, sizeof(dns_dtenv_t)); @@ -380,7 +337,7 @@ dns_dt_reopen(dns_dtenv_t *env, int roll) { (roll < 0) ? "reopening" : "rolling", env->path); - generation++; + atomic_fetch_add_release(&global_generation, 1); if (env->iothr != NULL) { fstrm_iothr_destroy(&env->iothr); @@ -474,49 +431,33 @@ dns_dt_setversion(dns_dtenv_t *env, const char *version) { return (toregion(env, &env->version, version)); } +static void +set_dt_ioq(unsigned int generation, struct fstrm_iothr_queue *ioq) { + dt_ioq.generation = generation; + dt_ioq.ioq = ioq; +} + static struct fstrm_iothr_queue * dt_queue(dns_dtenv_t *env) { - isc_result_t result; - struct ioq { - unsigned int generation; - struct fstrm_iothr_queue *ioq; - } *ioq; - REQUIRE(VALID_DTENV(env)); - if (env->iothr == NULL) - return (NULL); + unsigned int generation; - result = dt_init(); - if (result != ISC_R_SUCCESS) + if (env->iothr == NULL) { return (NULL); - - ioq = (struct ioq *)isc_thread_key_getspecific(dt_key); - if (ioq != NULL && ioq->generation != generation) { - result = isc_thread_key_setspecific(dt_key, NULL); - if (result != ISC_R_SUCCESS) - return (NULL); - free(ioq); - ioq = NULL; - } - if (ioq == NULL) { - ioq = malloc(sizeof(*ioq)); - if (ioq == NULL) - return (NULL); - ioq->generation = generation; - ioq->ioq = fstrm_iothr_get_input_queue(env->iothr); - if (ioq->ioq == NULL) { - free(ioq); - return (NULL); - } - result = isc_thread_key_setspecific(dt_key, ioq); - if (result != ISC_R_SUCCESS) { - free(ioq); - return (NULL); - } } - return (ioq->ioq); + generation = atomic_load_acquire(&global_generation); + if (dt_ioq.ioq != NULL && dt_ioq.generation != generation) { + set_dt_ioq(0, NULL); + } + if (dt_ioq.ioq == NULL) { + struct fstrm_iothr_queue *ioq = + fstrm_iothr_get_input_queue(env->iothr); + set_dt_ioq(generation, ioq); + } + + return (dt_ioq.ioq); } void @@ -547,7 +488,7 @@ destroy(dns_dtenv_t *env) { "closing dnstap"); env->magic = 0; - generation++; + atomic_fetch_add(&global_generation, 1); if (env->iothr != NULL) fstrm_iothr_destroy(&env->iothr); @@ -927,12 +868,6 @@ dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype, send_dt(view->dtenv, dm.buf, dm.len); } -void -dns_dt_shutdown() { - if (dt_mctx != NULL) - isc_mem_detach(&dt_mctx); -} - static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; diff --git a/lib/dns/include/dns/dnstap.h b/lib/dns/include/dns/dnstap.h index 2e948ebe24..601d3dabbe 100644 --- a/lib/dns/include/dns/dnstap.h +++ b/lib/dns/include/dns/dnstap.h @@ -267,13 +267,6 @@ dns_dt_getstats(dns_dtenv_t *env, isc_stats_t **statsp); *\li ISC_R_NOTFOUND */ -void -dns_dt_shutdown(void); -/*%< - * Shuts down dnstap and frees global resources. This function must only - * be called immediately before server shutdown. - */ - void dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype, isc_sockaddr_t *qaddr, isc_sockaddr_t *dstaddr, diff --git a/lib/dns/tests/dnstap_test.c b/lib/dns/tests/dnstap_test.c index 963f290e71..3c5d407cc1 100644 --- a/lib/dns/tests/dnstap_test.c +++ b/lib/dns/tests/dnstap_test.c @@ -132,8 +132,6 @@ create_test(void **state) { } cleanup(); - - dns_dt_shutdown(); } /* send dnstap messages */ @@ -263,7 +261,6 @@ send_test(void **state) { dns_dt_detach(&view->dtenv); dns_dt_detach(&dtenv); - dns_dt_shutdown(); dns_view_detach(&view); result = dns_dt_open(TAPFILE, dns_dtmode_file, dt_mctx, &handle); diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 2eef22497c..f90ef41113 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -354,7 +354,6 @@ dns_dt_send dns_dt_setidentity dns_dt_setupfile dns_dt_setversion -dns_dt_shutdown dns_dtdata_free @END NOTYET dns_dumpctx_attach From a4ffb6407331cbee1b61434c6f3e9c6ef106b30a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 4/8] Refactor the dns_geoip API to use ISC_THREAD_LOCAL Previously, the dns_geoip API used isc_thread_key API for TLS, which is fairly complicated and requires initialization of memory contexts, etc. This part of code was refactored to use a ISC_THREAD_LOCAL pointer which greatly simplifies the whole code related to storing TLS variables, and creating the local memory context was moved to named and stored in the named_g_geoip global context. --- bin/named/geoip.c | 10 ++- bin/named/include/named/geoip.h | 3 + bin/named/server.c | 6 +- lib/dns/geoip2.c | 124 ++++---------------------------- lib/dns/include/dns/geoip.h | 3 - lib/dns/tests/geoip_test.c | 18 ++--- lib/dns/win32/libdns.def.in | 1 - 7 files changed, 35 insertions(+), 130 deletions(-) diff --git a/bin/named/geoip.c b/bin/named/geoip.c index f67960fed5..d1824579e1 100644 --- a/bin/named/geoip.c +++ b/bin/named/geoip.c @@ -113,8 +113,7 @@ named_geoip_load(char *dir) { #endif } -void -named_geoip_shutdown(void) { +void named_geoip_unload(void) { #ifdef HAVE_GEOIP2 if (named_g_geoip->country != NULL) { MMDB_close(named_g_geoip->country); @@ -136,5 +135,12 @@ named_geoip_shutdown(void) { MMDB_close(named_g_geoip->domain); named_g_geoip->domain = NULL; } +#endif +} + +void +named_geoip_shutdown(void) { +#ifdef HAVE_GEOIP2 + named_geoip_unload(); #endif /* HAVE_GEOIP2 */ } diff --git a/bin/named/include/named/geoip.h b/bin/named/include/named/geoip.h index bb148e5b61..0b959fa23a 100644 --- a/bin/named/include/named/geoip.h +++ b/bin/named/include/named/geoip.h @@ -19,5 +19,8 @@ named_geoip_init(void); void named_geoip_load(char *dir); +void +named_geoip_unload(void); + void named_geoip_shutdown(void); diff --git a/bin/named/server.c b/bin/named/server.c index 400bd15b46..628ed0d28a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8342,7 +8342,8 @@ load_configuration(const char *filename, named_server_t *server, /* * Release any previously opened GeoIP2 databases. */ - named_geoip_shutdown(); + named_geoip_unload(); + /* * Initialize GeoIP databases from the configured location. * This should happen before configuring any ACLs, so that we @@ -9744,7 +9745,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { } #if defined(HAVE_GEOIP2) named_geoip_shutdown(); - dns_geoip_shutdown(); #endif /* HAVE_GEOIP2 */ dns_db_detach(&server->in_roothints); @@ -9866,7 +9866,7 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { /* * GeoIP must be initialized before the interface * manager (which includes the ACL environment) - * is created + * is created. */ named_geoip_init(); #endif /* HAVE_GEOIP2 */ diff --git a/lib/dns/geoip2.c b/lib/dns/geoip2.c index c0ab66a241..098a8844d5 100644 --- a/lib/dns/geoip2.c +++ b/lib/dns/geoip2.c @@ -64,7 +64,6 @@ */ typedef struct geoip_state { - isc_mem_t *mctx; uint16_t subtype; const MMDB_s *db; isc_netaddr_t addr; @@ -72,117 +71,28 @@ typedef struct geoip_state { MMDB_entry_s entry; } geoip_state_t; -static isc_mutex_t key_mutex; -static bool state_key_initialized = false; -static isc_thread_key_t state_key; -static isc_once_t mutex_once = ISC_ONCE_INIT; -static isc_mem_t *state_mctx = NULL; +ISC_THREAD_LOCAL geoip_state_t geoip_state = { 0 }; static void -key_mutex_init(void) { - isc_mutex_init(&key_mutex); -} - -static void -free_state(void *arg) { - geoip_state_t *state = arg; - if (state != NULL) { - isc_mem_putanddetach(&state->mctx, - state, sizeof(geoip_state_t)); - } - isc_thread_key_setspecific(state_key, NULL); -} - -static isc_result_t -state_key_init(void) { - isc_result_t result; - - result = isc_once_do(&mutex_once, key_mutex_init); - if (result != ISC_R_SUCCESS) { - return (result); - } - - if (!state_key_initialized) { - LOCK(&key_mutex); - if (!state_key_initialized) { - int ret; - - if (state_mctx == NULL) { - isc_mem_create(&state_mctx); - } - isc_mem_setname(state_mctx, "geoip_state", NULL); - isc_mem_setdestroycheck(state_mctx, false); - - ret = isc_thread_key_create(&state_key, free_state); - if (ret == 0) { - state_key_initialized = true; - } else { - result = ISC_R_FAILURE; - } - } - UNLOCK(&key_mutex); - } - - return (result); -} - -static isc_result_t set_state(const MMDB_s *db, const isc_netaddr_t *addr, - MMDB_lookup_result_s mmresult, MMDB_entry_s entry, - geoip_state_t **statep) + MMDB_lookup_result_s mmresult, MMDB_entry_s entry) { - geoip_state_t *state = NULL; - isc_result_t result; - - result = state_key_init(); - if (result != ISC_R_SUCCESS) { - return (result); - } - - state = (geoip_state_t *) isc_thread_key_getspecific(state_key); - if (state == NULL) { - state = isc_mem_get(state_mctx, sizeof(geoip_state_t)); - memset(state, 0, sizeof(*state)); - - result = isc_thread_key_setspecific(state_key, state); - if (result != ISC_R_SUCCESS) { - isc_mem_put(state_mctx, state, sizeof(geoip_state_t)); - return (result); - } - - isc_mem_attach(state_mctx, &state->mctx); - } - - state->db = db; - state->addr = *addr; - state->mmresult = mmresult; - state->entry = entry; - - if (statep != NULL) { - *statep = state; - } - - return (ISC_R_SUCCESS); + geoip_state.db = db; + geoip_state.addr = *addr; + geoip_state.mmresult = mmresult; + geoip_state.entry = entry; } static geoip_state_t * get_entry_for(MMDB_s * const db, const isc_netaddr_t *addr) { - isc_result_t result; isc_sockaddr_t sa; - geoip_state_t *state; MMDB_lookup_result_s match; int err; - result = state_key_init(); - if (result != ISC_R_SUCCESS) { - return (NULL); - } - - state = (geoip_state_t *) isc_thread_key_getspecific(state_key); - if (state != NULL) { - if (db == state->db && isc_netaddr_equal(addr, &state->addr)) { - return (state); - } + if (db == geoip_state.db && + isc_netaddr_equal(addr, &geoip_state.addr)) + { + return (&geoip_state); } isc_sockaddr_fromnetaddr(&sa, addr, 0); @@ -191,12 +101,9 @@ get_entry_for(MMDB_s * const db, const isc_netaddr_t *addr) { return (NULL); } - result = set_state(db, addr, match, match.entry, &state); - if (result != ISC_R_SUCCESS) { - return (NULL); - } + set_state(db, addr, match, match.entry); - return (state); + return (&geoip_state); } static dns_geoip_subtype_t @@ -482,10 +389,3 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, */ return (false); } - -void -dns_geoip_shutdown(void) { - if (state_mctx != NULL) { - isc_mem_detach(&state_mctx); - } -} diff --git a/lib/dns/include/dns/geoip.h b/lib/dns/include/dns/geoip.h index 4dc3291c62..223a457ef9 100644 --- a/lib/dns/include/dns/geoip.h +++ b/lib/dns/include/dns/geoip.h @@ -105,9 +105,6 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, const dns_geoip_databases_t *geoip, const dns_geoip_elem_t *elt); -void -dns_geoip_shutdown(void); - ISC_LANG_ENDDECLS #endif /* HAVE_GEOIP2 */ diff --git a/lib/dns/tests/geoip_test.c b/lib/dns/tests/geoip_test.c index 7481881a1e..21a887bae2 100644 --- a/lib/dns/tests/geoip_test.c +++ b/lib/dns/tests/geoip_test.c @@ -334,17 +334,17 @@ int main(void) { #if defined(HAVE_GEOIP2) const struct CMUnitTest tests[] = { - cmocka_unit_test_setup_teardown(country, _setup, _teardown), - cmocka_unit_test_setup_teardown(country_v6, _setup, _teardown), - cmocka_unit_test_setup_teardown(city, _setup, _teardown), - cmocka_unit_test_setup_teardown(city_v6, _setup, _teardown), - cmocka_unit_test_setup_teardown(asnum, _setup, _teardown), - cmocka_unit_test_setup_teardown(isp, _setup, _teardown), - cmocka_unit_test_setup_teardown(org, _setup, _teardown), - cmocka_unit_test_setup_teardown(domain, _setup, _teardown), + cmocka_unit_test(country), + cmocka_unit_test(country_v6), + cmocka_unit_test(city), + cmocka_unit_test(city_v6), + cmocka_unit_test(asnum), + cmocka_unit_test(isp), + cmocka_unit_test(org), + cmocka_unit_test(domain), }; - return (cmocka_run_group_tests(tests, NULL, NULL)); + return (cmocka_run_group_tests(tests, _setup, _teardown)); #else print_message("1..0 # Skip GeoIP not enabled\n"); #endif diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index f90ef41113..d77368debb 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -384,7 +384,6 @@ dns_generalstats_dump dns_generalstats_increment @IF GEOIP dns_geoip_match -dns_geoip_shutdown @END GEOIP dns_hashalg_fromtext dns_ipkeylist_clear From 5d43b7126cea37d9109040dd3363ca42896b4ae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 5/8] Refactor the irs_context API to use ISC_THREAD_LOCAL Previously, the irs_context API used isc_thread_key API for TLS, which is fairly complicated and requires initialization of memory contexts, etc. This part of code was refactored to use a ISC_THREAD_LOCAL pointer which greatly simplifies the whole code related to storing TLS variables. --- lib/irs/context.c | 64 ++++------------------------------------------- 1 file changed, 5 insertions(+), 59 deletions(-) diff --git a/lib/irs/context.c b/lib/irs/context.c index d1b71c4204..de2796b4a6 100644 --- a/lib/irs/context.c +++ b/lib/irs/context.c @@ -42,11 +42,7 @@ #define DNS_CONF "/etc/dns.conf" #endif -static bool thread_key_initialized = false; -static isc_mutex_t thread_key_mutex; -static isc_thread_key_t irs_context_key; -static isc_once_t once = ISC_ONCE_INIT; - +ISC_THREAD_LOCAL irs_context_t *irs_context = NULL; struct irs_context { /* @@ -119,68 +115,20 @@ ctxs_init(isc_mem_t **mctxp, isc_appctx_t **actxp, return (result); } -static void -free_specific_context(void *arg) { - irs_context_t *context = arg; - - irs_context_destroy(&context); - - isc_thread_key_setspecific(irs_context_key, NULL); -} - -static void -thread_key_mutex_init(void) { - isc_mutex_init(&thread_key_mutex); -} - -static isc_result_t -thread_key_init(void) { - isc_result_t result; - - result = isc_once_do(&once, thread_key_mutex_init); - if (result != ISC_R_SUCCESS) - return (result); - - if (!thread_key_initialized) { - LOCK(&thread_key_mutex); - - if (!thread_key_initialized && - isc_thread_key_create(&irs_context_key, - free_specific_context) != 0) { - result = ISC_R_FAILURE; - } else - thread_key_initialized = true; - - UNLOCK(&thread_key_mutex); - } - - return (result); -} - isc_result_t irs_context_get(irs_context_t **contextp) { - irs_context_t *context; isc_result_t result; REQUIRE(contextp != NULL && *contextp == NULL); - result = thread_key_init(); - if (result != ISC_R_SUCCESS) - return (result); - - context = isc_thread_key_getspecific(irs_context_key); - if (context == NULL) { - result = irs_context_create(&context); - if (result != ISC_R_SUCCESS) - return (result); - result = isc_thread_key_setspecific(irs_context_key, context); + if (irs_context == NULL) { + result = irs_context_create(&irs_context); if (result != ISC_R_SUCCESS) { - irs_context_destroy(&context); return (result); } } - *contextp = context; + *contextp = irs_context; return (ISC_R_SUCCESS); } @@ -289,6 +237,7 @@ irs_context_destroy(irs_context_t **contextp) { REQUIRE(contextp != NULL); context = *contextp; REQUIRE(IRS_CONTEXT_VALID(context)); + *contextp = irs_context = NULL; isc_task_detach(&context->task); irs_dnsconf_destroy(&context->dnsconf); @@ -302,9 +251,6 @@ irs_context_destroy(irs_context_t **contextp) { isc_mem_putanddetach(&context->mctx, context, sizeof(*context)); - *contextp = NULL; - - (void)isc_thread_key_setspecific(irs_context_key, NULL); } isc_mem_t * From b1a7ec7481e619c8b38bf592c11fb72191b54665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH 6/8] Remove isc_thread_key API in favor of ISC_THREAD_LOCAL variables For BIND 9.16+, TLS aware compiler is required, and using ISC_THREAD_LOCAL is preferred way of using Thread Local Storage. The isc_thread_key API is no longer used anywhere and hence was removed from BIND 9. --- lib/isc/pthreads/include/isc/thread.h | 6 ------ lib/isc/win32/include/isc/thread.h | 13 ------------- lib/isc/win32/libisc.def.in | 4 ---- lib/isc/win32/thread.c | 22 ---------------------- 4 files changed, 45 deletions(-) diff --git a/lib/isc/pthreads/include/isc/thread.h b/lib/isc/pthreads/include/isc/thread.h index ac50cce0fd..4f4d592ef5 100644 --- a/lib/isc/pthreads/include/isc/thread.h +++ b/lib/isc/pthreads/include/isc/thread.h @@ -30,7 +30,6 @@ typedef pthread_t isc_thread_t; typedef void * isc_threadresult_t; typedef void * isc_threadarg_t; typedef isc_threadresult_t (*isc_threadfunc_t)(isc_threadarg_t); -typedef pthread_key_t isc_thread_key_t; void isc_thread_create(isc_threadfunc_t, isc_threadarg_t, isc_thread_t *); @@ -53,11 +52,6 @@ isc_thread_setaffinity(int cpu); #define isc_thread_self \ (unsigned long)pthread_self -#define isc_thread_key_create pthread_key_create -#define isc_thread_key_getspecific pthread_getspecific -#define isc_thread_key_setspecific pthread_setspecific -#define isc_thread_key_delete pthread_key_delete - /*** *** Thread-Local Storage ***/ diff --git a/lib/isc/win32/include/isc/thread.h b/lib/isc/win32/include/isc/thread.h index 05225f9016..844c30e976 100644 --- a/lib/isc/win32/include/isc/thread.h +++ b/lib/isc/win32/include/isc/thread.h @@ -61,7 +61,6 @@ typedef HANDLE isc_thread_t; typedef DWORD isc_threadresult_t; typedef void * isc_threadarg_t; typedef isc_threadresult_t (WINAPI *isc_threadfunc_t)(isc_threadarg_t); -typedef DWORD isc_thread_key_t; #define isc_thread_self (unsigned long)GetCurrentThreadId @@ -82,18 +81,6 @@ isc_thread_setname(isc_thread_t, const char *); isc_result_t isc_thread_setaffinity(int cpu); -int -isc_thread_key_create(isc_thread_key_t *key, void (*func)(void *)); - -int -isc_thread_key_delete(isc_thread_key_t key); - -void * -isc_thread_key_getspecific(isc_thread_key_t); - -int -isc_thread_key_setspecific(isc_thread_key_t key, void *value); - #define isc_thread_yield() Sleep(0) #if HAVE___DECLSPEC_THREAD diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index e65855f187..129f84b253 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -651,10 +651,6 @@ isc_taskpool_setprivilege isc_taskpool_size isc_thread_create isc_thread_join -isc_thread_key_create -isc_thread_key_delete -isc_thread_key_getspecific -isc_thread_key_setspecific isc_thread_setaffinity isc_thread_setconcurrency isc_thread_setname diff --git a/lib/isc/win32/thread.c b/lib/isc/win32/thread.c index 6917f9c464..22722b07de 100644 --- a/lib/isc/win32/thread.c +++ b/lib/isc/win32/thread.c @@ -73,25 +73,3 @@ isc_thread_setaffinity(int cpu) { /* no-op on Windows for now */ return (ISC_R_SUCCESS); } - -void * -isc_thread_key_getspecific(isc_thread_key_t key) { - return(TlsGetValue(key)); -} - -int -isc_thread_key_setspecific(isc_thread_key_t key, void *value) { - return (TlsSetValue(key, value) ? 0 : GetLastError()); -} - -int -isc_thread_key_create(isc_thread_key_t *key, void (*func)(void *)) { - *key = TlsAlloc(); - - return ((*key != -1) ? 0 : GetLastError()); -} - -int -isc_thread_key_delete(isc_thread_key_t key) { - return (TlsFree(key) ? 0 : GetLastError()); -} From c62748c9e31ea464b34f295dbd960073f6416fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Dec 2019 06:30:16 +0100 Subject: [PATCH 7/8] Update PLATFORMS.md to explicitly list Thread Local Storage as requirement for BIND 9 --- PLATFORMS | 40 +++++++++++++++++++++++++--------------- PLATFORMS.md | 14 +++++++++++--- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/PLATFORMS b/PLATFORMS index 56b45c9357..6dfdad4d81 100644 --- a/PLATFORMS +++ b/PLATFORMS @@ -5,16 +5,28 @@ Supported platforms In general, this version of BIND will build and run on any POSIX-compliant system with a C11-compliant C compiler, BSD-style sockets with RFC-compliant IPv6 support, POSIX-compliant threads, the libuv -asynchronous I/O library, and the OpenSSL cryptography library. Atomic -operations support from the compiler is needed, either in the form of -builtin operations, C11 atomics, or the Interlocked family of functions on -Windows. +asynchronous I/O library, and the OpenSSL cryptography library. -BIND 9.15 requires fairly recent version of libuv library to run (>= 1.x). -For some of the older systems listed below, you will have to install -updated libuv package from sources such as EPEL, PPA and other native -sources for updated packages. The other option is to install libuv from -sources. +The following C11 features are used in BIND 9: + + * Atomic operations support from the compiler is needed, either in the + form of builtin operations, C11 atomics, or the Interlocked family of + functions on Windows. + + * Thread Local Storage support from the compiler is needed, either in + the form of C11 _Thread_local/thread_local, the __thread GCC + extension, or the __declspec(thread) MSVC extension on Windows. + +BIND 9.15 requires a fairly recent version of libuv (at least 1.x). For +some of the older systems listed below, you will have to install an +updated libuv package from sources such as EPEL, PPA, or other native +sources for updated packages. The other option is to build and install +libuv from source. + +Certain optional BIND features have additional library dependencies. These +include libxml2 and libjson-c for statistics, libmaxminddb for +geolocation, libfstrm and libprotobuf-c for DNSTAP, and libidn2 for +internationalized domain name conversion. ISC regularly tests BIND on many operating systems and architectures, but lacks the resources to test all of them. Consequently, ISC is only able to @@ -58,10 +70,10 @@ Server 2012 R2, none of these are tested regularly by ISC. Community maintained -These systems may not all have easily available the required dependencies -for building BIND although it will be possible in many cases to compile +These systems may not all have the required dependencies for building BIND +easily available, although it will be possible in many cases to compile those directly from source. The community and interested parties may wish -to help with maintenance and we welcome patch contributions, although we +to help with maintenance, and we welcome patch contributions, although we cannot guarantee that we will accept them. All contributions will be assessed against the risk of adverse effect on officially supported platforms. @@ -84,6 +96,4 @@ These are platforms on which BIND 9.15 is known not to build or run: * Platforms that don't support atomic operations (via compiler or library) * Linux without NPTL (Native POSIX Thread Library) - * Platforms where libuv cannot be compiled - -Platform quirks + * Platforms on which libuv cannot be compiled diff --git a/PLATFORMS.md b/PLATFORMS.md index 52041d07a7..cb94bc63d9 100644 --- a/PLATFORMS.md +++ b/PLATFORMS.md @@ -13,9 +13,17 @@ In general, this version of BIND will build and run on any POSIX-compliant system with a C11-compliant C compiler, BSD-style sockets with RFC-compliant IPv6 support, POSIX-compliant threads, the `libuv` asynchronous I/O library, -and the OpenSSL cryptography library. Atomic operations support from the -compiler is needed, either in the form of builtin operations, C11 atomics, -or the `Interlocked` family of functions on Windows. +and the OpenSSL cryptography library. + +The following C11 features are used in BIND 9: + +* Atomic operations support from the compiler is needed, either in the form of + builtin operations, C11 atomics, or the `Interlocked` family of functions on + Windows. + +* Thread Local Storage support from the compiler is needed, either in the form + of C11 `_Thread_local`/`thread_local`, the `__thread` GCC extension, or + the `__declspec(thread)` MSVC extension on Windows. BIND 9.15 requires a fairly recent version of `libuv` (at least 1.x). For some of the older systems listed below, you will have to install an updated From 04e901a86c8a07830185aa93768316ffaeace868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Dec 2019 22:10:22 +0100 Subject: [PATCH 8/8] Add CHANGES --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index cd3cc7231d..0e1a46cabf 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5331. [func] Use compiler-provided mechanisms for thread local + storage, and make the requirement for such mechanisms + explicit in configure. [GL #1444] + 5330. [bug] 'configure --without-python' was ineffective if PYTHON was set in the environment. [GL #1434]