Merge branch 'ondrej/cleanup-isc_thread-api' into 'master'

Cleanup the isc_thread API

See merge request isc-projects/bind9!2159

bin/tests/optional/rwlock_test.c

@@ -109,18 +109,16 @@ main(int argc, char *argv[]) {
 		snprintf(name, sizeof(name), "%02u", i);
 		dupname = strdup(name);
 		RUNTIME_CHECK(dupname != NULL);
-		if (i != 0 && i % 3 == 0)
-			RUNTIME_CHECK(isc_thread_create(run1, dupname,
-							&workers[i]) ==
-			       ISC_R_SUCCESS);
-		else
-			RUNTIME_CHECK(isc_thread_create(run2, dupname,
-							&workers[i]) ==
-			       ISC_R_SUCCESS);
+		if (i != 0 && i % 3 == 0) {
+			isc_thread_create(run1, dupname, &workers[i]);
+		} else {
+			isc_thread_create(run2, dupname, &workers[i]);
+		}
 	}
 
-	for (i = 0; i < nworkers; i++)
-		(void)isc_thread_join(workers[i], NULL);
+	for (i = 0; i < nworkers; i++) {
+		isc_thread_join(workers[i], NULL);
+	}
 
 	isc_rwlock_destroy(&lock);
 

lib/dns/tests/name_test.c

@@ -743,13 +743,11 @@ benchmark_test(void **state) {
 	nthreads = ISC_MIN(isc_os_ncpus(), 32);
 	nthreads = ISC_MAX(nthreads, 1);
 	for (i = 0; i < nthreads; i++) {
-		result = isc_thread_create(fromwire_thread, NULL, &threads[i]);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_create(fromwire_thread, NULL, &threads[i]);
 	}
 
 	for (i = 0; i < nthreads; i++) {
-		result = isc_thread_join(threads[i], NULL);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_join(threads[i], NULL);
 	}
 
 	result = isc_time_now(&ts2);

lib/dns/tests/rbt_test.c

@@ -1295,13 +1295,11 @@ benchmark(void **state) {
 	nthreads = ISC_MIN(isc_os_ncpus(), 32);
 	nthreads = ISC_MAX(nthreads, 1);
 	for (i = 0; i < nthreads; i++) {
-		result = isc_thread_create(find_thread, mytree, &threads[i]);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_create(find_thread, mytree, &threads[i]);
 	}
 
 	for (i = 0; i < nthreads; i++) {
-		result = isc_thread_join(threads[i], NULL);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_join(threads[i], NULL);
 	}
 
 	result = isc_time_now(&ts2);

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

@@ -32,9 +32,12 @@ typedef void * isc_threadarg_t;
 typedef isc_threadresult_t (*isc_threadfunc_t)(isc_threadarg_t);
 typedef pthread_key_t isc_thread_key_t;
 
-isc_result_t
+void
 isc_thread_create(isc_threadfunc_t, isc_threadarg_t, isc_thread_t *);
 
+void
+isc_thread_join(isc_thread_t thread, isc_threadresult_t *result);
+
 void
 isc_thread_setconcurrency(unsigned int level);
 
@@ -47,12 +50,6 @@ isc_thread_setname(isc_thread_t thread, const char *name);
 isc_result_t
 isc_thread_setaffinity(int cpu);
 
-/* XXX We could do fancier error handling... */
-
-#define isc_thread_join(t, rp) \
-	((pthread_join((t), (rp)) == 0) ? \
-	 ISC_R_SUCCESS : ISC_R_UNEXPECTED)
-
 #define isc_thread_self \
 	(unsigned long)pthread_self
 

lib/isc/pthreads/thread.c

@@ -27,6 +27,7 @@
 #include <sys/procset.h>
 #endif
 
+#include <isc/strerr.h>
 #include <isc/thread.h>
 #include <isc/util.h>
 
@@ -34,7 +35,16 @@
 #define THREAD_MINSTACKSIZE		(1024U * 1024)
 #endif
 
-isc_result_t
+#define _FATAL(r, f)							\
+	{								\
+		char strbuf[ISC_STRERRORSIZE];				\
+		strerror_r(r, strbuf, sizeof(strbuf));			\
+		isc_error_fatal(__FILE__, __LINE__,			\
+				f " failed: %s", \
+				strbuf);				\
+	}
+
+void
 isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg,
 		  isc_thread_t *thread)
 {
@@ -50,23 +60,35 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg,
 #if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \
     defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE)
 	ret = pthread_attr_getstacksize(&attr, &stacksize);
-	if (ret != 0)
-		return (ISC_R_UNEXPECTED);
+	if (ret != 0) {
+		_FATAL(ret, "pthread_attr_getstacksize()");
+	}
 
 	if (stacksize < THREAD_MINSTACKSIZE) {
 		ret = pthread_attr_setstacksize(&attr, THREAD_MINSTACKSIZE);
-		if (ret != 0)
-			return (ISC_R_UNEXPECTED);
+		if (ret != 0) {
+			_FATAL(ret, "pthread_attr_setstacksize()");
+		}
 	}
 #endif
 
 	ret = pthread_create(thread, &attr, func, arg);
-	if (ret != 0)
-		return (ISC_R_UNEXPECTED);
+	if (ret != 0) {
+		_FATAL(ret, "pthread_create()");
+	}
 
 	pthread_attr_destroy(&attr);
 
-	return (ISC_R_SUCCESS);
+	return;
+}
+
+void
+isc_thread_join(isc_thread_t thread, isc_threadresult_t *result)
+{
+	int ret = pthread_join(thread, result);
+	if (ret != 0) {
+		_FATAL(ret, "pthread_join()");
+	}
 }
 
 #ifdef __NetBSD__

lib/isc/task.c

@@ -1384,9 +1384,8 @@ isc_taskmgr_create(isc_mem_t *mctx, unsigned int workers,
 
 		manager->queues[i].manager = manager;
 		manager->queues[i].threadid = i;
-		RUNTIME_CHECK(isc_thread_create(run, &manager->queues[i],
-						&manager->queues[i].thread)
-			      == ISC_R_SUCCESS);
+		isc_thread_create(run, &manager->queues[i],
+				  &manager->queues[i].thread);
 		char name[21];
 		snprintf(name, sizeof(name), "isc-worker%04u", i);
 		isc_thread_setname(manager->queues[i].thread, name);
@@ -1484,8 +1483,9 @@ isc_taskmgr_destroy(isc_taskmgr_t **managerp) {
 	/*
 	 * Wait for all the worker threads to exit.
 	 */
-	for (i = 0; i < manager->workers; i++)
-		(void)isc_thread_join(manager->queues[i].thread, NULL);
+	for (i = 0; i < manager->workers; i++) {
+		isc_thread_join(manager->queues[i].thread, NULL);
+	}
 
 	manager_free(manager);
 

lib/isc/tests/mem_test.c

@@ -463,13 +463,11 @@ isc_mem_benchmark(void **state) {
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	for (int i = 0; i < nthreads; i++) {
-		result = isc_thread_create(mem_thread, &size, &threads[i]);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_create(mem_thread, &size, &threads[i]);
 		size = size / 2;
 	}
 	for (int i = 0; i < nthreads; i++) {
-		result = isc_thread_join(threads[i], NULL);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_join(threads[i], NULL);
 	}
 
 	result = isc_time_now(&ts2);
@@ -527,13 +525,11 @@ isc_mempool_benchmark(void **state) {
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	for (int i = 0; i < nthreads; i++) {
-		result = isc_thread_create(mempool_thread, mp, &threads[i]);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_create(mempool_thread, mp, &threads[i]);
 		size = size / 2;
 	}
 	for (int i = 0; i < nthreads; i++) {
-		result = isc_thread_join(threads[i], NULL);
-		assert_int_equal(result, ISC_R_SUCCESS);
+		isc_thread_join(threads[i], NULL);
 	}
 
 	result = isc_time_now(&ts2);

lib/isc/timer.c

@@ -697,17 +697,7 @@ isc_timermgr_create(isc_mem_t *mctx, isc_timermgr_t **managerp) {
 	isc_mutex_init(&manager->lock);
 	isc_mem_attach(mctx, &manager->mctx);
 	isc_condition_init(&manager->wakeup);
-	if (isc_thread_create(run, manager, &manager->thread) !=
-	    ISC_R_SUCCESS) {
-		isc_mem_detach(&manager->mctx);
-		(void)isc_condition_destroy(&manager->wakeup);
-		isc_mutex_destroy(&manager->lock);
-		isc_heap_destroy(&manager->heap);
-		isc_mem_put(mctx, manager, sizeof(*manager));
-		UNEXPECTED_ERROR(__FILE__, __LINE__, "%s",
-				 "isc_thread_create() failed");
-		return (ISC_R_UNEXPECTED);
-	}
+	isc_thread_create(run, manager, &manager->thread);
 	isc_thread_setname(manager->thread, "isc-timer");
 
 	*managerp = (isc_timermgr_t *)manager;
@@ -749,9 +739,7 @@ isc_timermgr_destroy(isc_timermgr_t **managerp) {
 	/*
 	 * Wait for thread to exit.
 	 */
-	if (isc_thread_join(manager->thread, NULL) != ISC_R_SUCCESS)
-		UNEXPECTED_ERROR(__FILE__, __LINE__, "%s",
-				 "isc_thread_join() failed");
+	isc_thread_join(manager->thread, NULL);
 
 	/*
 	 * Clean up.

lib/isc/unix/socket.c

@@ -3810,10 +3810,9 @@ isc_socketmgr_create2(isc_mem_t *mctx, isc_socketmgr_t **managerp,
 		manager->threads[i].manager = manager;
 		manager->threads[i].threadid = i;
 		setup_thread(&manager->threads[i]);
-		RUNTIME_CHECK(isc_thread_create(netthread,
-						&manager->threads[i],
-						&manager->threads[i].thread)
-			      == ISC_R_SUCCESS);
+		isc_thread_create(netthread,
+				  &manager->threads[i],
+				  &manager->threads[i].thread);
 		char tname[1024];
 		sprintf(tname, "isc-socket-%d", i);
 		isc_thread_setname(manager->threads[i].thread, tname);
@@ -3885,12 +3884,7 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) {
 	 * Wait for thread to exit.
 	 */
 	for (int i = 0; i < manager->nthreads; i++) {
-		isc_result_t result;
-		result = isc_thread_join(manager->threads[i].thread, NULL);
-		if (result != ISC_R_SUCCESS) {
-			UNEXPECTED_ERROR(__FILE__, __LINE__,
-					 "isc_thread_join() failed");
-		}
+		isc_thread_join(manager->threads[i].thread, NULL);
 		cleanup_thread(manager->mctx, &manager->threads[i]);
 	}
 	/*

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

@@ -67,10 +67,10 @@ typedef DWORD isc_thread_key_t;
 
 ISC_LANG_BEGINDECLS
 
-isc_result_t
+void
 isc_thread_create(isc_threadfunc_t, isc_threadarg_t, isc_thread_t *);
 
-isc_result_t
+void
 isc_thread_join(isc_thread_t, isc_threadresult_t *);
 
 void

lib/isc/win32/socket.c

@@ -2569,10 +2569,7 @@ isc_socketmgr_destroy(isc_socketmgr_t **managerp) {
 	 * Wait for threads to exit.
 	 */
 	for (int i = 0; i < manager->maxIOCPThreads; i++) {
-		if (isc_thread_join((isc_thread_t) manager->hIOCPThreads[i],
-			NULL) != ISC_R_SUCCESS)
-			UNEXPECTED_ERROR(__FILE__, __LINE__,
-					 "isc_thread_join() for Completion Port failed");
+		isc_thread_join((isc_thread_t) manager->hIOCPThreads[i], NULL);
 	}
 	/*
 	 * Clean up.

lib/isc/win32/thread.c

@@ -11,10 +11,11 @@
 
 #include <process.h>
 
+#include <isc/strerr.h>
 #include <isc/thread.h>
 #include <isc/util.h>
 
-isc_result_t
+void
 isc_thread_create(isc_threadfunc_t start, isc_threadarg_t arg,
 		  isc_thread_t *threadp)
 {
@@ -23,27 +24,30 @@ isc_thread_create(isc_threadfunc_t start, isc_threadarg_t arg,
 
 	thread = (isc_thread_t)_beginthreadex(NULL, 0, start, arg, 0, &id);
 	if (thread == NULL) {
-		/* XXX */
-		return (ISC_R_UNEXPECTED);
+		char strbuf[ISC_STRERRORSIZE];
+		strerror_r(errno, strbuf, sizeof(strbuf));
+		isc_error_fatal(__FILE__, __LINE__, "_beginthreadex failed: %s",
+				strbuf);
 	}
 
 	*threadp = thread;
 
-	return (ISC_R_SUCCESS);
+	return;
 }
 
-isc_result_t
+void
 isc_thread_join(isc_thread_t thread, isc_threadresult_t *rp) {
 	DWORD result;
 
 	result = WaitForSingleObject(thread, INFINITE);
 	if (result != WAIT_OBJECT_0) {
-		/* XXX */
-		return (ISC_R_UNEXPECTED);
+		isc_error_fatal(__FILE__, __LINE__,
+				"WaitForSingleObject() != WAIT_OBJECT_0");
 	}
 	if (rp != NULL && !GetExitCodeThread(thread, rp)) {
-		/* XXX */
-		return (ISC_R_UNEXPECTED);
+		isc_error_fatal(__FILE__, __LINE__,
+				"GetExitCodeThread() failed: %d", GetLastError());
+
 	}
 	(void)CloseHandle(thread);