From: Tony Finch <fanf@isc.org>
Date: Tue, 4 Apr 2023 16:40:39 +0000 (+0100)
Subject: Simplify isc_thread a little
X-Git-Tag: v9.19.13~14^2~3
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e0248bf60f1d8924aa87ee53a0f66c58b94225ab;p=thirdparty%2Fbind9.git

Simplify isc_thread a little

Remove the `isc_threadarg_t` and `isc_threadresult_t`
typedefs which were unhelpful disguises for `void *`,
and free the dummy jemalloc allocation sooner.
---

diff --git a/lib/isc/include/isc/thread.h b/lib/isc/include/isc/thread.h
index 63496165d61..f433d063ac1 100644
--- a/lib/isc/include/isc/thread.h
+++ b/lib/isc/include/isc/thread.h
@@ -33,15 +33,13 @@
 ISC_LANG_BEGINDECLS
 
 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 void *(*isc_threadfunc_t)(void *);
 
 void
-isc_thread_create(isc_threadfunc_t, isc_threadarg_t, isc_thread_t *);
+isc_thread_create(isc_threadfunc_t, void *, isc_thread_t *);
 
 void
-isc_thread_join(isc_thread_t thread, isc_threadresult_t *result);
+isc_thread_join(isc_thread_t thread, void **);
 
 void
 isc_thread_yield(void);
diff --git a/lib/isc/loop.c b/lib/isc/loop.c
index fa00feb4d3e..de8073c8355 100644
--- a/lib/isc/loop.c
+++ b/lib/isc/loop.c
@@ -330,8 +330,8 @@ loop_close(isc_loop_t *loop) {
 	isc_mem_detach(&loop->mctx);
 }
 
-static isc_threadresult_t
-loop_thread(isc_threadarg_t arg) {
+static void *
+loop_thread(void *arg) {
 	isc_loop_t *loop = (isc_loop_t *)arg;
 
 	/* Initialize the thread_local variable */
@@ -340,7 +340,7 @@ loop_thread(isc_threadarg_t arg) {
 
 	loop_run(loop);
 
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 
 void
diff --git a/lib/isc/thread.c b/lib/isc/thread.c
index 7d2c81872f2..c3bcc397e82 100644
--- a/lib/isc/thread.c
+++ b/lib/isc/thread.c
@@ -49,23 +49,38 @@
 
 struct thread_wrap {
 	isc_threadfunc_t func;
-	isc_threadarg_t arg;
-	isc_threadresult_t result;
-	void *jemalloc_enforce_init;
+	void *arg;
+	void (*free)(void *);
 };
 
-static isc_threadresult_t
-thread_run(isc_threadarg_t arg) {
+static struct thread_wrap *
+thread_wrap(isc_threadfunc_t func, void *arg) {
+	struct thread_wrap *wrap = malloc(sizeof(*wrap));
+
+	RUNTIME_CHECK(wrap != NULL);
+	*wrap = (struct thread_wrap){
+		.free = free, /* from stdlib */
+		.func = func,
+		.arg = arg,
+	};
+
+	return (wrap);
+}
+
+static void *
+thread_run(void *arg) {
 	struct thread_wrap *wrap = arg;
+	isc_threadfunc_t wrap_func = wrap->func;
+	void *wrap_arg = wrap->arg;
+	void *result = NULL;
 
 	/*
-	 * Ensure every thread starts with a malloc() call to prevent memory
-	 * bloat caused by a jemalloc quirk.  While this dummy allocation is
-	 * not used for anything, free() must not be immediately called for it
-	 * so that an optimizing compiler does not strip away such a pair of
-	 * malloc() + free() calls altogether, as it would foil the fix.
+	 * Every thread starts with a malloc() call to prevent memory bloat
+	 * caused by a jemalloc quirk. To stop an optimizing compiler from
+	 * stripping out free(malloc(1)), we call free via a function pointer.
 	 */
-	wrap->jemalloc_enforce_init = malloc(8);
+	wrap->free(malloc(1));
+	wrap->free(wrap);
 
 	/* Re-seed the random number generator in each thread. */
 	isc__random_initialize();
@@ -74,38 +89,24 @@ thread_run(isc_threadarg_t arg) {
 	isc__iterated_hash_initialize();
 
 	/* Run the main function */
-	wrap->result = wrap->func(wrap->arg);
+	result = wrap_func(wrap_arg);
 
 	/* Cleanup */
 	isc__iterated_hash_shutdown();
 
-	/* Return the wrapper struct for jemalloc cleanup */
-	return (wrap);
+	return (result);
 }
 
 void
-isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg,
-		  isc_thread_t *thread) {
-	pthread_attr_t attr;
-	struct thread_wrap *wrap = malloc(sizeof(*wrap));
-	RUNTIME_CHECK(wrap != NULL);
-
-	*wrap = (struct thread_wrap){
-		.func = func,
-		.arg = arg,
-	};
-
-#if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \
-	defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE)
-	size_t stacksize;
-#endif /* if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \
-	* defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) */
+isc_thread_create(isc_threadfunc_t func, void *arg, isc_thread_t *thread) {
 	int ret;
+	pthread_attr_t attr;
 
 	pthread_attr_init(&attr);
 
 #if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \
 	defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE)
+	size_t stacksize;
 	ret = pthread_attr_getstacksize(&attr, &stacksize);
 	PTHREADS_RUNTIME_CHECK(pthread_attr_getstacksize, ret);
 
@@ -116,27 +117,17 @@ 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, thread_run, wrap);
+	ret = pthread_create(thread, &attr, thread_run, thread_wrap(func, arg));
 	PTHREADS_RUNTIME_CHECK(pthread_create, ret);
 
 	pthread_attr_destroy(&attr);
-
-	return;
 }
 
 void
-isc_thread_join(isc_thread_t thread, isc_threadresult_t *result) {
-	void *wrap_v;
-	int ret = pthread_join(thread, &wrap_v);
+isc_thread_join(isc_thread_t thread, void **resultp) {
+	int ret = pthread_join(thread, resultp);
 
 	PTHREADS_RUNTIME_CHECK(pthread_join, ret);
-
-	struct thread_wrap *wrap = wrap_v;
-	if (result != NULL) {
-		*result = wrap->result;
-	}
-	free(wrap->jemalloc_enforce_init);
-	free(wrap);
 }
 
 void
diff --git a/tests/isc/mem_test.c b/tests/isc/mem_test.c
index 6680ed4adf5..4e80db0111a 100644
--- a/tests/isc/mem_test.c
+++ b/tests/isc/mem_test.c
@@ -490,8 +490,8 @@ ISC_RUN_TEST_IMPL(isc_mem_traceflag) {
 
 static atomic_size_t mem_size;
 
-static isc_threadresult_t
-mem_thread(isc_threadarg_t arg) {
+static void *
+mem_thread(void *arg) {
 	isc_mem_t *mctx2 = (isc_mem_t *)arg;
 	void *items[NUM_ITEMS];
 	size_t size = atomic_load(&mem_size);
@@ -508,7 +508,7 @@ mem_thread(isc_threadarg_t arg) {
 		}
 	}
 
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 
 ISC_RUN_TEST_IMPL(isc_mem_benchmark) {
diff --git a/tests/isc/quota_test.c b/tests/isc/quota_test.c
index cbf2724ea29..8efa9232c75 100644
--- a/tests/isc/quota_test.c
+++ b/tests/isc/quota_test.c
@@ -232,7 +232,7 @@ static void *
 quota_release(void *arg) {
 	uv_sleep(10);
 	isc_quota_release((isc_quota_t *)arg);
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 
 static void
@@ -243,7 +243,7 @@ quota_callback(void *data) {
 	isc_thread_create(quota_release, &quota, &g_threads[tnum]);
 }
 
-static isc_threadresult_t
+static void *
 quota_thread(void *qtip) {
 	qthreadinfo_t *qti = (qthreadinfo_t *)qtip;
 	for (int i = 0; i < 100; i++) {
@@ -257,7 +257,7 @@ quota_thread(void *qtip) {
 					  &g_threads[tnum]);
 		}
 	}
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 
 ISC_RUN_TEST_IMPL(isc_quota_callback_mt) {
diff --git a/tests/isc/spinlock_test.c b/tests/isc/spinlock_test.c
index 9c3dde53da8..a7eaae9ae45 100644
--- a/tests/isc/spinlock_test.c
+++ b/tests/isc/spinlock_test.c
@@ -87,8 +87,8 @@ static size_t expected_counter = SIZE_MAX;
 #if HAD_PTHREAD_SPIN_INIT
 static pthread_spinlock_t spin;
 
-static isc_threadresult_t
-pthread_spin_thread(isc_threadarg_t arg) {
+static void *
+pthread_spin_thread(void *arg) {
 	size_t cont = *(size_t *)arg;
 
 	for (size_t i = 0; i < loops; i++) {
@@ -100,14 +100,14 @@ pthread_spin_thread(isc_threadarg_t arg) {
 		isc_pause_n(cont);
 	}
 
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 #endif
 
 static isc_spinlock_t lock;
 
-static isc_threadresult_t
-isc_spinlock_thread(isc_threadarg_t arg) {
+static void *
+isc_spinlock_thread(void *arg) {
 	size_t cont = *(size_t *)arg;
 
 	for (size_t i = 0; i < loops; i++) {
@@ -119,7 +119,7 @@ isc_spinlock_thread(isc_threadarg_t arg) {
 		isc_pause_n(cont);
 	}
 
-	return ((isc_threadresult_t)0);
+	return (NULL);
 }
 
 ISC_RUN_TEST_IMPL(isc_spinlock_benchmark) {