Merge branch '2396-add-thread-trampoline-for-thread-accounting' into 'main'

Resolve "BIND 9.16 unit tests failing reliably on x86_64 NUMA machines"

Closes #2396

See merge request isc-projects/bind9!4687

CHANGES

@@ -1,3 +1,10 @@
+5592.	[bug]		Add globally available thread_id (isc_tid_v) that's
+			incremented for each new thread, but the old thread
+			ids are reused, so the maximum thread_id always
+			correspond to the maximum number of threads running
+			at the time. This fixes the hazard pointer tables
+			overflow on machines with many cores. [GL #2396]
+
 5591.	[bug]		Fix a crash happening when "stale-answer-client-timeout"
 			is triggered and there is no (stale) data for it in the
 			cache. [GL #2503]

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	\

lib/isc/hp.c

@@ -62,12 +62,6 @@ static int isc__hp_max_threads = HP_MAX_THREADS;
 /* Maximum number of retired objects per thread */
 static int isc__hp_max_retired = HP_MAX_THREADS * HP_MAX_HPS;
 
-#define TID_UNKNOWN -1
-
-static atomic_int_fast32_t tid_v_base = ATOMIC_VAR_INIT(0);
-
-static thread_local int tid_v = TID_UNKNOWN;
-
 typedef struct retirelist {
 	int size;
 	uintptr_t *list;
@@ -83,12 +77,7 @@ struct isc_hp {
 
 static inline int
 tid(void) {
-	if (tid_v == TID_UNKNOWN) {
+	return (isc_tid_v);
-		tid_v = atomic_fetch_add(&tid_v_base, 1);
-		REQUIRE(tid_v < isc__hp_max_threads);
-	}
-
-	return (tid_v);
 }
 
 void

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();
 }

lib/isc/pthreads/include/isc/thread.h

@@ -25,6 +25,8 @@
 #include <isc/lang.h>
 #include <isc/result.h>
 
+extern thread_local size_t isc_tid_v;
+
 ISC_LANG_BEGINDECLS
 
 typedef pthread_t isc_thread_t;
@@ -50,6 +52,6 @@ isc_thread_setname(isc_thread_t thread, const char *name);
 isc_result_t
 isc_thread_setaffinity(int cpu);
 
-#define isc_thread_self (unsigned long)pthread_self
+#define isc_thread_self (uintptr_t) pthread_self
 
 ISC_LANG_ENDDECLS

lib/isc/pthreads/thread.c

@@ -30,6 +30,8 @@
 #include <isc/thread.h>
 #include <isc/util.h>
 
+#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()");
 	}

lib/isc/rwlock.c

@@ -175,7 +175,7 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
 static void
 print_lock(const char *operation, isc_rwlock_t *rwl, isc_rwlocktype_t type) {
 	fprintf(stderr,
-		"rwlock %p thread %lu %s(%s): "
+		"rwlock %p thread %" PRIuPTR " %s(%s): "
 		"write_requests=%u, write_completions=%u, "
 		"cnt_and_flag=0x%x, readers_waiting=%u, "
 		"write_granted=%u, write_quota=%u\n",

lib/isc/task.c

@@ -61,13 +61,14 @@
  */
 
 #ifdef ISC_TASK_TRACE
-#define XTRACE(m)                                                            \
+#define XTRACE(m)                                                  \
-	fprintf(stderr, "task %p thread %lu: %s\n", task, isc_thread_self(), \
+	fprintf(stderr, "task %p thread %" PRIuPTR ": %s\n", task, \
-		(m))
+		isc_thread_self(), (m))
-#define XTTRACE(t, m) \
+#define XTTRACE(t, m)                                             \
-	fprintf(stderr, "task %p thread %lu: %s\n", (t), isc_thread_self(), (m))
+	fprintf(stderr, "task %p thread %" PRIuPTR ": %s\n", (t), \
+		isc_thread_self(), (m))
 #define XTHREADTRACE(m) \
-	fprintf(stderr, "thread %lu: %s\n", isc_thread_self(), (m))
+	fprintf(stderr, "thread %" PRIuPTR ": %s\n", isc_thread_self(), (m))
 #else /* ifdef ISC_TASK_TRACE */
 #define XTRACE(m)
 #define XTTRACE(t, m)

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 <inttypes.h>
+#include <stdlib.h>
+
+#include <isc/mem.h>
+#include <isc/mutex.h>
+#include <isc/once.h>
+#include <isc/thread.h>
+#include <isc/util.h>
+
+#include "trampoline_p.h"
+
+#define ISC__TRAMPOLINE_UNUSED 0
+
+struct isc__trampoline {
+	int tid; /* const */
+	uintptr_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);
+}

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 <isc/thread.h>
+
+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())
+ */

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;

lib/isc/win32/condition.c

@@ -93,7 +93,7 @@ register_thread(unsigned long thrd, isc_condition_t *gblcond,
 }
 
 static isc_result_t
-find_thread_condition(unsigned long thrd, isc_condition_t *cond,
+find_thread_condition(uintptr_t thrd, isc_condition_t *cond,
 		      isc_condition_thread_t **threadcondp) {
 	isc_condition_thread_t *threadcond;
 

lib/isc/win32/include/isc/condition.h

@@ -22,8 +22,8 @@
 typedef struct isc_condition_thread isc_condition_thread_t;
 
 struct isc_condition_thread {
-	unsigned long th;
+	uintptr_t th;
-	HANDLE	      handle[2];
+	HANDLE	  handle[2];
 	ISC_LINK(isc_condition_thread_t) link;
 };
 

lib/isc/win32/include/isc/thread.h

@@ -12,11 +12,14 @@
 #ifndef ISC_THREAD_H
 #define ISC_THREAD_H 1
 
+#include <inttypes.h>
 #include <windows.h>
 
 #include <isc/lang.h>
 #include <isc/result.h>
 
+extern thread_local size_t isc_tid_v;
+
 /*
  * Inlines to help with wait return checking
  */
@@ -66,7 +69,7 @@ typedef DWORD  isc_threadresult_t;
 typedef void * isc_threadarg_t;
 typedef isc_threadresult_t(WINAPI *isc_threadfunc_t)(isc_threadarg_t);
 
-#define isc_thread_self (unsigned long)GetCurrentThreadId
+#define isc_thread_self (uintptr_t) GetCurrentThreadId
 
 ISC_LANG_BEGINDECLS
 

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

lib/isc/win32/libisc.vcxproj.filters.in

@@ -358,6 +358,9 @@
     <ClInclude Include="..\openssl_shim.h">
       <Filter>Win32 Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="..\trampoline_p.h">
+      <Filter>Win32 Header Files</Filter>
+    </ClInclude>
     <ClInclude Include="errno2result.h">
       <Filter>Win32 Header Files</Filter>
     </ClInclude>
@@ -614,6 +617,9 @@
     <ClCompile Include="..\tlsstream.c">
       <Filter>Library Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="..\trampoline.c">
+      <Filter>Library Source Files</Filter>
+    </ClCompile>
     <ClCompile Include="..\tm.c">
       <Filter>Library Source Files</Filter>
     </ClCompile>

lib/isc/win32/libisc.vcxproj.in

@@ -378,6 +378,7 @@ copy InstallFiles ..\Build\Release\
     <ClInclude Include="..\entropy_private.h" />
     <ClInclude Include="..\fsaccess_common_p.h" />
     <ClInclude Include="..\openssl_shim.h" />
+    <ClInclude Include="..\trampoline_p.h" />
     <ClInclude Include="syslog.h" />
     <ClInclude Include="unistd.h" />
     <ClInclude Include="..\..\versions.h" />
@@ -449,6 +450,7 @@ copy InstallFiles ..\Build\Release\
     <ClCompile Include="..\task.c" />
     <ClCompile Include="..\taskpool.c" />
     <ClCompile Include="..\timer.c" />
+    <ClCompile Include="..\trampoline.c" />
     <ClCompile Include="..\tls.c" />
     <ClCompile Include="..\tm.c" />
     <ClCompile Include="..\url.c" />

lib/isc/win32/thread.c

@@ -11,17 +11,25 @@
 
 #include <process.h>
 
+#include <isc/mutex.h>
+#include <isc/once.h>
 #include <isc/strerr.h>
 #include <isc/thread.h>
 #include <isc/util.h>
 
+#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));

lib/ns/query.c

@@ -161,18 +161,17 @@ client_trace(ns_client_t *client, int level, const char *message) {
 					     sizeof(tbuf));
 			isc_log_write(ns_lctx, NS_LOGCATEGORY_CLIENT,
 				      NS_LOGMODULE_QUERY, level,
-				      "query client=%p thread=0x%lx "
+				      "query client=%p thread=0x%" PRIxPTR
 				      "(%s/%s): %s",
-				      client, (unsigned long)isc_thread_self(),
+				      client, isc_thread_self(), qbuf, tbuf,
-				      qbuf, tbuf, message);
+				      message);
 		}
 	} else {
 		isc_log_write(ns_lctx, NS_LOGCATEGORY_CLIENT,
 			      NS_LOGMODULE_QUERY, level,
-			      "query client=%p thread=0x%lx "
+			      "query client=%p thread=0x%" PRIxPTR
 			      "(<unknown-query>): %s",
-			      client, (unsigned long)isc_thread_self(),
+			      client, isc_thread_self(), message);
-			      message);
 	}
 }
 #define CTRACE(l, m)  client_trace(client, l, m)

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