From cbbecfcc822d7607adfcfcdaa35ccad05fa972c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 16 Feb 2021 15:57:39 +0100 Subject: [PATCH] Add isc_trampoline API to have simple accounting around threads The current isc_hp API uses internal tid_v variable that gets incremented for each new thread using hazard pointers. This tid_v variable is then used as a index to global shared table with hazard pointers state. Since the tid_v is only incremented and never decremented the table could overflow very quickly if we create set of threads for short period of time, they finish the work and cease to exist. Then we create identical set of threads and so on and so on. This is not a problem for a normal `named` operation as the set of threads is stable, but the problematic place are the unit tests where we test network manager or other APIs (task, timer) that create threads. This commits adds a thin wrapper around any function called from isc_thread_create() that adds unique-but-reusable small digit thread id that can be used as index to f.e. hazard pointer tables. The trampoline wrapper ensures that the thread ids will be reused, so the highest thread_id number doesn't grow indefinitely when threads are created and destroyed and then created again. This fixes the hazard pointer table overflow on machines with many cores. [GL #2396] --- lib/isc/Makefile.am | 2 + lib/isc/lib.c | 3 + lib/isc/pthreads/include/isc/thread.h | 2 + lib/isc/pthreads/thread.c | 9 +- lib/isc/trampoline.c | 189 ++++++++++++++++++++++++ lib/isc/trampoline_p.h | 53 +++++++ lib/isc/win32/DLLMain.c | 3 + lib/isc/win32/include/isc/thread.h | 2 + lib/isc/win32/libisc.def.in | 5 + lib/isc/win32/libisc.vcxproj.filters.in | 6 + lib/isc/win32/libisc.vcxproj.in | 2 + lib/isc/win32/thread.c | 10 +- util/copyrights | 2 + 13 files changed, 286 insertions(+), 2 deletions(-) create mode 100644 lib/isc/trampoline.c create mode 100644 lib/isc/trampoline_p.h diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index 3d2e891db2..79f71c1b85 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -213,6 +213,8 @@ libisc_la_SOURCES = \ taskpool.c \ timer.c \ tls.c \ + trampoline.c \ + trampoline_p.h \ tm.c \ utf8.c \ pthreads/condition.c \ diff --git a/lib/isc/lib.c b/lib/isc/lib.c index dad744903d..b1afa2b355 100644 --- a/lib/isc/lib.c +++ b/lib/isc/lib.c @@ -19,6 +19,7 @@ #include "mem_p.h" #include "tls_p.h" +#include "trampoline_p.h" /*** *** Functions @@ -38,10 +39,12 @@ void isc__initialize(void) { isc__mem_initialize(); isc__tls_initialize(); + isc__trampoline_initialize(); } void isc__shutdown(void) { + isc__trampoline_shutdown(); isc__tls_shutdown(); isc__mem_shutdown(); } diff --git a/lib/isc/pthreads/include/isc/thread.h b/lib/isc/pthreads/include/isc/thread.h index 9712b298a5..66c6e1c439 100644 --- a/lib/isc/pthreads/include/isc/thread.h +++ b/lib/isc/pthreads/include/isc/thread.h @@ -25,6 +25,8 @@ #include #include +extern thread_local size_t isc_tid_v; + ISC_LANG_BEGINDECLS typedef pthread_t isc_thread_t; diff --git a/lib/isc/pthreads/thread.c b/lib/isc/pthreads/thread.c index 6477aa876f..21a3cd8029 100644 --- a/lib/isc/pthreads/thread.c +++ b/lib/isc/pthreads/thread.c @@ -30,6 +30,8 @@ #include #include +#include "trampoline_p.h" + #ifndef THREAD_MINSTACKSIZE #define THREAD_MINSTACKSIZE (1024U * 1024) #endif /* ifndef THREAD_MINSTACKSIZE */ @@ -45,6 +47,10 @@ void isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, isc_thread_t *thread) { pthread_attr_t attr; + isc__trampoline_t *trampoline_arg; + + trampoline_arg = isc__trampoline_get(func, arg); + #if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \ defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) size_t stacksize; @@ -70,7 +76,8 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, #endif /* if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \ * defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) */ - ret = pthread_create(thread, &attr, func, arg); + ret = pthread_create(thread, &attr, isc__trampoline_run, + trampoline_arg); if (ret != 0) { _FATAL(ret, "pthread_create()"); } diff --git a/lib/isc/trampoline.c b/lib/isc/trampoline.c new file mode 100644 index 0000000000..c8ce5a4cad --- /dev/null +++ b/lib/isc/trampoline.c @@ -0,0 +1,189 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +/*! \file */ + +#include +#include + +#include +#include +#include +#include +#include + +#include "trampoline_p.h" + +#define ISC__TRAMPOLINE_UNUSED 0 + +struct isc__trampoline { + int tid; /* const */ + isc_thread_t self; + isc_threadfunc_t start; + isc_threadarg_t arg; +}; + +static isc_once_t isc__trampoline_initialize_once = ISC_ONCE_INIT; +static isc_once_t isc__trampoline_shutdown_once = ISC_ONCE_INIT; +static isc_mutex_t isc__trampoline_lock; +static isc__trampoline_t **trampolines; +thread_local size_t isc_tid_v = SIZE_MAX; +static size_t isc__trampoline_min = 1; +static size_t isc__trampoline_max = 65; + +/* + * We can't use isc_mem API here, because it's called too + * early and when the isc_mem_debugging flags are changed + * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are + * added, neither isc_mem_put() nor isc_mem_free() can be used + * to free up the memory allocated here because the flags were + * not set when calling isc_mem_get() or isc_mem_allocate() + * here. + * + * Actually, since this is a single allocation at library load + * and deallocation at library unload, using the standard + * allocator without the tracking is fine for this purpose. + */ +static isc__trampoline_t * +isc__trampoline_new(int tid, isc_threadfunc_t start, isc_threadarg_t arg) { + isc__trampoline_t *trampoline = calloc(1, sizeof(*trampoline)); + RUNTIME_CHECK(trampoline != NULL); + + *trampoline = (isc__trampoline_t){ + .tid = tid, + .start = start, + .arg = arg, + .self = ISC__TRAMPOLINE_UNUSED, + }; + + return (trampoline); +} + +static void +trampoline_initialize(void) { + isc_mutex_init(&isc__trampoline_lock); + + trampolines = calloc(isc__trampoline_max, sizeof(trampolines[0])); + RUNTIME_CHECK(trampolines != NULL); + + /* Get the trampoline slot 0 for the main thread */ + trampolines[0] = isc__trampoline_new(0, NULL, NULL); + trampolines[0]->self = isc_thread_self(); + isc_tid_v = trampolines[0]->tid; + + /* Initialize the other trampolines */ + for (size_t i = 1; i < isc__trampoline_max; i++) { + trampolines[i] = NULL; + } + isc__trampoline_min = 1; +} + +void +isc__trampoline_initialize(void) { + isc_result_t result = isc_once_do(&isc__trampoline_initialize_once, + trampoline_initialize); + RUNTIME_CHECK(result == ISC_R_SUCCESS); +} + +static void +trampoline_shutdown(void) { + /* + * When the program using the library exits abruptly and the library + * gets unloaded, there might be some existing trampolines from unjoined + * threads. We intentionally ignore those and don't check whether all + * trampolines have been cleared before exiting. + */ + free(trampolines[0]); + free(trampolines); + trampolines = NULL; + isc_mutex_destroy(&isc__trampoline_lock); +} + +void +isc__trampoline_shutdown(void) { + isc_result_t result = isc_once_do(&isc__trampoline_shutdown_once, + trampoline_shutdown); + RUNTIME_CHECK(result == ISC_R_SUCCESS); +} + +isc__trampoline_t * +isc__trampoline_get(isc_threadfunc_t start, isc_threadarg_t arg) { + isc__trampoline_t **tmp = NULL; + isc__trampoline_t *trampoline = NULL; + LOCK(&isc__trampoline_lock); +again: + for (size_t i = isc__trampoline_min; i < isc__trampoline_max; i++) { + if (trampolines[i] == NULL) { + trampoline = isc__trampoline_new(i, start, arg); + trampolines[i] = trampoline; + isc__trampoline_min = i + 1; + goto done; + } + } + tmp = calloc(2 * isc__trampoline_max, sizeof(trampolines[0])); + RUNTIME_CHECK(tmp != NULL); + for (size_t i = 0; i < isc__trampoline_max; i++) { + tmp[i] = trampolines[i]; + } + for (size_t i = isc__trampoline_max; i < 2 * isc__trampoline_max; i++) { + tmp[i] = NULL; + } + free(trampolines); + trampolines = tmp; + isc__trampoline_max = isc__trampoline_max * 2; + goto again; +done: + INSIST(trampoline != NULL); + UNLOCK(&isc__trampoline_lock); + + return (trampoline); +} + +static void +trampoline_put(isc__trampoline_t *trampoline) { + LOCK(&isc__trampoline_lock); + REQUIRE(trampoline->tid > 0 && + (size_t)trampoline->tid < isc__trampoline_max); + REQUIRE(trampoline->self == isc_thread_self()); + REQUIRE(trampolines[trampoline->tid] == trampoline); + + trampolines[trampoline->tid] = NULL; + + if (isc__trampoline_min > (size_t)trampoline->tid) { + isc__trampoline_min = trampoline->tid; + } + + free(trampoline); + + UNLOCK(&isc__trampoline_lock); + return; +} + +isc_threadresult_t +isc__trampoline_run(isc_threadarg_t arg) { + isc__trampoline_t *trampoline = (isc__trampoline_t *)arg; + isc_threadresult_t result; + + REQUIRE(trampoline->tid > 0 && + (size_t)trampoline->tid < isc__trampoline_max); + REQUIRE(trampoline->self == ISC__TRAMPOLINE_UNUSED); + + /* Initialize the trampoline */ + isc_tid_v = trampoline->tid; + trampoline->self = isc_thread_self(); + + /* Run the main function */ + result = (trampoline->start)(trampoline->arg); + + trampoline_put(trampoline); + + return (result); +} diff --git a/lib/isc/trampoline_p.h b/lib/isc/trampoline_p.h new file mode 100644 index 0000000000..a089a97139 --- /dev/null +++ b/lib/isc/trampoline_p.h @@ -0,0 +1,53 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +#include + +typedef struct isc__trampoline isc__trampoline_t; + +void +isc__trampoline_initialize(void); +/*%< + * Initialize the thread trampoline internal structures, must be called only + * once as a library constructor (see lib/isc/lib.c). + */ + +void +isc__trampoline_shutdown(void); +/*%< + * Destroy the thread trampoline internal structures, must be called only + * once as a library destructor (see lib/isc/lib.c). + */ + +isc__trampoline_t * +isc__trampoline_get(isc_threadfunc_t start_routine, isc_threadarg_t arg); +/*%< + * Get a free thread trampoline structure and initialize it with + * start_routine and arg passed to start_routine. + * + * Requires: + *\li 'start_routine' is a valid non-NULL thread start_routine + */ + +isc_threadresult_t +isc__trampoline_run(isc_threadarg_t arg); +/*%< + * Run the thread trampoline, this will get passed to the actual + * pthread_create() (or Windows equivalent), initialize the isc_tid_v. + * + * Requires: + *\li 'arg' is a valid isc_trampoline_t + * + * Returns: + *\li return value from start_routine (see isc__trampoline_get()) + */ diff --git a/lib/isc/win32/DLLMain.c b/lib/isc/win32/DLLMain.c index 0ae5177198..187e84fbcd 100644 --- a/lib/isc/win32/DLLMain.c +++ b/lib/isc/win32/DLLMain.c @@ -18,6 +18,7 @@ #include "mem_p.h" #include "tls_p.h" +#include "trampoline_p.h" /* * Called when we enter the DLL @@ -32,6 +33,7 @@ __declspec(dllexport) BOOL WINAPI case DLL_PROCESS_ATTACH: isc__mem_initialize(); isc__tls_initialize(); + isc__trampoline_initialize(); break; /* @@ -39,6 +41,7 @@ __declspec(dllexport) BOOL WINAPI * termination or a call to FreeLibrary. */ case DLL_PROCESS_DETACH: + isc__trampoline_shutdown(); isc__tls_shutdown(); isc__mem_shutdown(); break; diff --git a/lib/isc/win32/include/isc/thread.h b/lib/isc/win32/include/isc/thread.h index 7964e0a650..49776b8daa 100644 --- a/lib/isc/win32/include/isc/thread.h +++ b/lib/isc/win32/include/isc/thread.h @@ -17,6 +17,8 @@ #include #include +extern thread_local size_t isc_tid_v; + /* * Inlines to help with wait return checking */ diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 5797632b87..3662fd57ab 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -715,6 +715,10 @@ isc__tls_shutdown isc_tlsctx_createclient isc_tlsctx_createserver isc_tlsctx_free +isc__trampoline_initialize +isc__trampoline_shutdown +isc__trampoline_get +isc__trampoline_run isc_tm_timegm isc_tm_strptime isc_url_parse @@ -796,6 +800,7 @@ isc_commandline_reset DATA isc_dscp_check_value DATA isc_hashctx DATA isc_mem_debugging DATA +isc_tid_v DATA @IF PKCS11 pk11_verbose_init DATA @END PKCS11 diff --git a/lib/isc/win32/libisc.vcxproj.filters.in b/lib/isc/win32/libisc.vcxproj.filters.in index aab1a899ae..c859f3e46d 100644 --- a/lib/isc/win32/libisc.vcxproj.filters.in +++ b/lib/isc/win32/libisc.vcxproj.filters.in @@ -358,6 +358,9 @@ Win32 Header Files + + Win32 Header Files + Win32 Header Files @@ -614,6 +617,9 @@ Library Source Files + + Library Source Files + Library Source Files diff --git a/lib/isc/win32/libisc.vcxproj.in b/lib/isc/win32/libisc.vcxproj.in index 7d989c5591..856f43391a 100644 --- a/lib/isc/win32/libisc.vcxproj.in +++ b/lib/isc/win32/libisc.vcxproj.in @@ -378,6 +378,7 @@ copy InstallFiles ..\Build\Release\ + @@ -449,6 +450,7 @@ copy InstallFiles ..\Build\Release\ + diff --git a/lib/isc/win32/thread.c b/lib/isc/win32/thread.c index 814fc9dc0d..9e9707b8bd 100644 --- a/lib/isc/win32/thread.c +++ b/lib/isc/win32/thread.c @@ -11,17 +11,25 @@ #include +#include +#include #include #include #include +#include "trampoline_p.h" + void isc_thread_create(isc_threadfunc_t start, isc_threadarg_t arg, isc_thread_t *threadp) { isc_thread_t thread; unsigned int id; + isc__trampoline_t *trampoline_arg; - thread = (isc_thread_t)_beginthreadex(NULL, 0, start, arg, 0, &id); + trampoline_arg = isc__trampoline_get(start, arg); + + thread = (isc_thread_t)_beginthreadex(NULL, 0, isc__trampoline_run, + trampoline_arg, 0, &id); if (thread == NULL) { char strbuf[ISC_STRERRORSIZE]; strerror_r(errno, strbuf, sizeof(strbuf)); diff --git a/util/copyrights b/util/copyrights index ee79ea4c98..89617bf07f 100644 --- a/util/copyrights +++ b/util/copyrights @@ -1996,6 +1996,8 @@ ./lib/isc/tls.c C 2021 ./lib/isc/tls_p.h C 2021 ./lib/isc/tm.c C 2014,2016,2018,2019,2020,2021 +./lib/isc/trampoline.c C 2021 +./lib/isc/trampoline_p.h C 2021 ./lib/isc/unix/dir.c C 1999,2000,2001,2004,2005,2007,2008,2009,2011,2012,2016,2017,2018,2019,2020,2021 ./lib/isc/unix/errno.c C 2016,2018,2019,2020,2021 ./lib/isc/unix/errno2result.c C 2000,2001,2002,2004,2005,2007,2011,2012,2013,2016,2018,2019,2020,2021