From: Ondřej Surý Date: Thu, 18 Jul 2019 15:47:40 +0000 (+0200) Subject: Make isc_thread_join() assert internally on failure X-Git-Tag: v9.15.3~17^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46919579bb5846b7a5ad1deb9ab66fc3fa3aa440;p=thirdparty%2Fbind9.git Make isc_thread_join() assert internally on failure Previously isc_thread_join() would return ISC_R_UNEXPECTED on a failure to create new thread. All such occurences were caught and wrapped into assert function at higher level. The function was simplified to assert directly in the isc_thread_join() function and all caller level assertions were removed. --- diff --git a/bin/tests/optional/rwlock_test.c b/bin/tests/optional/rwlock_test.c index d7a87ac92a2..4d42ac4338d 100644 --- a/bin/tests/optional/rwlock_test.c +++ b/bin/tests/optional/rwlock_test.c @@ -116,8 +116,9 @@ main(int argc, char *argv[]) { } } - 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); diff --git a/lib/dns/tests/name_test.c b/lib/dns/tests/name_test.c index 4e51b9300fc..ff728225981 100644 --- a/lib/dns/tests/name_test.c +++ b/lib/dns/tests/name_test.c @@ -747,8 +747,7 @@ benchmark_test(void **state) { } 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); diff --git a/lib/dns/tests/rbt_test.c b/lib/dns/tests/rbt_test.c index 34119f4dd03..7c2657698ca 100644 --- a/lib/dns/tests/rbt_test.c +++ b/lib/dns/tests/rbt_test.c @@ -1299,8 +1299,7 @@ benchmark(void **state) { } 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); diff --git a/lib/isc/pthreads/include/isc/thread.h b/lib/isc/pthreads/include/isc/thread.h index a3a6106753f..63194d3b043 100644 --- a/lib/isc/pthreads/include/isc/thread.h +++ b/lib/isc/pthreads/include/isc/thread.h @@ -35,6 +35,9 @@ typedef pthread_key_t isc_thread_key_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 diff --git a/lib/isc/pthreads/thread.c b/lib/isc/pthreads/thread.c index 6e4ebc4c0a0..4831cedc65c 100644 --- a/lib/isc/pthreads/thread.c +++ b/lib/isc/pthreads/thread.c @@ -27,6 +27,7 @@ #include #endif +#include #include #include @@ -39,7 +40,7 @@ char strbuf[ISC_STRERRORSIZE]; \ strerror_r(r, strbuf, sizeof(strbuf)); \ isc_error_fatal(__FILE__, __LINE__, \ - f # " failed: %s", \ + f " failed: %s", \ strbuf); \ } @@ -60,20 +61,20 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) ret = pthread_attr_getstacksize(&attr, &stacksize); if (ret != 0) { - _FATAL(ret, "pthread_attr_getstacksize"); + _FATAL(ret, "pthread_attr_getstacksize()"); } if (stacksize < THREAD_MINSTACKSIZE) { ret = pthread_attr_setstacksize(&attr, THREAD_MINSTACKSIZE); if (ret != 0) { - _FATAL(ret, pthread_attr_setstacksize); + _FATAL(ret, "pthread_attr_setstacksize()"); } } #endif ret = pthread_create(thread, &attr, func, arg); if (ret != 0) { - _FATAL(ret,"pthread_create"); + _FATAL(ret, "pthread_create()"); } pthread_attr_destroy(&attr); @@ -81,6 +82,15 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, 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__ #define pthread_setconcurrency(a) (void) a/* nothing */ #endif diff --git a/lib/isc/task.c b/lib/isc/task.c index 6199dc64fcd..6405335951a 100644 --- a/lib/isc/task.c +++ b/lib/isc/task.c @@ -1483,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); diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index 735d4116f0b..0181b02ae81 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -467,8 +467,7 @@ isc_mem_benchmark(void **state) { 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); @@ -530,8 +529,7 @@ isc_mempool_benchmark(void **state) { 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); diff --git a/lib/isc/timer.c b/lib/isc/timer.c index 276c2f5566f..f7027f56258 100644 --- a/lib/isc/timer.c +++ b/lib/isc/timer.c @@ -739,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. diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c index f9ed0cc9860..39e5a5fbe25 100644 --- a/lib/isc/unix/socket.c +++ b/lib/isc/unix/socket.c @@ -3884,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]); } /* diff --git a/lib/isc/win32/include/isc/thread.h b/lib/isc/win32/include/isc/thread.h index 5642b974fb8..f091d0623c4 100644 --- a/lib/isc/win32/include/isc/thread.h +++ b/lib/isc/win32/include/isc/thread.h @@ -70,7 +70,7 @@ ISC_LANG_BEGINDECLS 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 diff --git a/lib/isc/win32/socket.c b/lib/isc/win32/socket.c index 54b76a72046..4f78c2d40ca 100644 --- a/lib/isc/win32/socket.c +++ b/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. diff --git a/lib/isc/win32/thread.c b/lib/isc/win32/thread.c index 9d421b21489..6917f9c4648 100644 --- a/lib/isc/win32/thread.c +++ b/lib/isc/win32/thread.c @@ -25,8 +25,7 @@ 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) { char strbuf[ISC_STRERRORSIZE]; - /* FIXME */ - strerror_r(GetLastError(), strbuf, sizeof(strbuf)); + strerror_r(errno, strbuf, sizeof(strbuf)); isc_error_fatal(__FILE__, __LINE__, "_beginthreadex failed: %s", strbuf); } @@ -36,18 +35,19 @@ isc_thread_create(isc_threadfunc_t start, isc_threadarg_t arg, 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);