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 d7a87ac92a273e9d6602d8a80676386ea66a766c..4d42ac4338d12109ab8b430d7bb40a99f870b4e4 100644 (file)
--- 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 4e51b9300fcf4b43c52c94830a8676bfce438eca..ff72822598108c5c8df48895e964efd882ca5ece 100644 (file)
--- 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 34119f4dd038d834e051f6b1c5040595b1637977..7c2657698ca1ea1139fa58a64bffd2ae1387097f 100644 (file)
--- 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 a3a6106753fecc88d985b96a027b7a876721dd8f..63194d3b043b3d2f0b70f590c169d5be96f2fbd6 100644 (file)
--- 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 6e4ebc4c0a05d2db9d0ff63ea7fc3ffcdc49e264..4831cedc65cf673af6142d06001b35f00a206301 100644 (file)
--- a/lib/isc/pthreads/thread.c
+++ b/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>
 
@@ -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 6199dc64fcd72beb61d259bdfcb87aa90a9fb104..6405335951a3ba128e70c4d1fe6a1a6fe091aa67 100644 (file)
--- 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 735d4116f0ba84a555a76738d591724ed1d2b91e..0181b02ae816f3b839885598a7c731f02cab6b1c 100644 (file)
--- 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 276c2f5566f2732e3c33a0632e0955e88f4d5ea7..f7027f5625833c1d2e4585b9990957713acebd27 100644 (file)
--- 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 f9ed0cc9860f1714ab4fb7d8d72649f19321e2a1..39e5a5fbe2597795572522d9c36a8ba5c94dec3b 100644 (file)
--- 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 5642b974fb836e21a692b7e13959e68e0a47e5c3..f091d0623c47d5e8d5e8a1715d7a0ac405fe02a2 100644 (file)
--- 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 54b76a72046771adb7d1ca9a3221fea916eeed97..4f78c2d40ca24563994c17b637e4427432faa5cd 100644 (file)
--- 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 9d421b214897e2da87832c32414fedd9fb827a44..6917f9c46484fb4371856ac5eb49dfb607693c4a 100644 (file)
--- 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);