Directly cause assertion failure on pthreads primitives failure

Instead of returning error values from isc_rwlock_*(), isc_mutex_*(),
and isc_condition_*() macros/functions and subsequently carrying out
runtime assertion checks on the return values in the calling code,
trigger assertion failures directly in those macros/functions whenever
any pthread function returns an error, as there is no point in
continuing execution in such a case anyway.

diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c
index 9b5e1c730982f7553315a3942011837ee02aea29..8ef933b16d85ade26f3f05ceda89a3e6ea719d11 100644 (file)
--- a/bin/dig/dighost.c
+++ b/bin/dig/dighost.c
@@ -160,18 +160,16 @@ dig_lookup_t *current_lookup = NULL;
  * Apply and clear locks at the event level in global task.
  * Can I get rid of these using shutdown events?  XXX
  */
-#define LOCK_LOOKUP                                                       \
-       {                                                                 \
-               debug("lock_lookup %s:%d", __FILE__, __LINE__);           \
-               check_result(isc_mutex_lock((&lookup_lock)), "isc_mutex_" \
-                                                            "lock");     \
-               debug("success");                                         \
-       }
-#define UNLOCK_LOOKUP                                                       \
-       {                                                                   \
-               debug("unlock_lookup %s:%d", __FILE__, __LINE__);           \
-               check_result(isc_mutex_unlock((&lookup_lock)), "isc_mutex_" \
-                                                              "unlock");   \
+#define LOCK_LOOKUP                                             \
+       {                                                       \
+               debug("lock_lookup %s:%d", __FILE__, __LINE__); \
+               isc_mutex_lock((&lookup_lock));                 \
+               debug("success");                               \
+       }
+#define UNLOCK_LOOKUP                                             \
+       {                                                         \
+               debug("unlock_lookup %s:%d", __FILE__, __LINE__); \
+               isc_mutex_unlock((&lookup_lock));                 \
        }
 
 static void

diff --git a/lib/isc/include/isc/condition.h b/lib/isc/include/isc/condition.h
index ca75f1069535454384ec8c46d49951a7f6074e2b..69622d32ced5ba2e4e22ce3c8f81807896d95433 100644 (file)
--- a/lib/isc/include/isc/condition.h
+++ b/lib/isc/include/isc/condition.h
@@ -33,18 +33,15 @@ typedef pthread_cond_t isc_condition_t;
                ERRNO_CHECK(pthread_cond_init, _ret);     \
        }
 
-#define isc_condition_wait(cp, mp)                            \
-       ((pthread_cond_wait((cp), (mp)) == 0) ? ISC_R_SUCCESS \
-                                             : ISC_R_UNEXPECTED)
+#define isc_condition_wait(cp, mp) \
+       RUNTIME_CHECK(pthread_cond_wait((cp), (mp)) == 0)
 
-#define isc_condition_signal(cp) \
-       ((pthread_cond_signal((cp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
+#define isc_condition_signal(cp) RUNTIME_CHECK(pthread_cond_signal((cp)) == 0)
 
 #define isc_condition_broadcast(cp) \
-       ((pthread_cond_broadcast((cp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
+       RUNTIME_CHECK(pthread_cond_broadcast((cp)) == 0)
 
-#define isc_condition_destroy(cp) \
-       ((pthread_cond_destroy((cp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
+#define isc_condition_destroy(cp) RUNTIME_CHECK(pthread_cond_destroy((cp)) == 0)
 
 ISC_LANG_BEGINDECLS
 

diff --git a/lib/isc/include/isc/mutex.h b/lib/isc/include/isc/mutex.h
index 8a8562bd8c4af0403e6511774934c07180882c1c..09d192f610bbfd1cf974a9dd6e0c40fa59fc6bcb 100644 (file)
--- a/lib/isc/include/isc/mutex.h
+++ b/lib/isc/include/isc/mutex.h
@@ -30,11 +30,9 @@ isc__mutex_init(isc_mutex_t *mp);
 
 #define isc_mutex_init(mp) isc__mutex_init((mp))
 
-#define isc_mutex_lock(mp) \
-       ((pthread_mutex_lock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
+#define isc_mutex_lock(mp) RUNTIME_CHECK(pthread_mutex_lock((mp)) == 0)
 
-#define isc_mutex_unlock(mp) \
-       ((pthread_mutex_unlock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
+#define isc_mutex_unlock(mp) RUNTIME_CHECK(pthread_mutex_unlock((mp)) == 0)
 
 #define isc_mutex_trylock(mp) \
        ((pthread_mutex_trylock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_LOCKBUSY)

diff --git a/lib/isc/include/isc/rwlock.h b/lib/isc/include/isc/rwlock.h
index a0d7083ca4659df67815caa943e784f3d56fb6a8..ba0ba0d43d9bfe75c5f8c274c3a701aeaff70f1f 100644 (file)
--- a/lib/isc/include/isc/rwlock.h
+++ b/lib/isc/include/isc/rwlock.h
@@ -82,13 +82,13 @@ void
 isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota,
                unsigned int write_quota);
 
-isc_result_t
+void
 isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
 
 isc_result_t
 isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
 
-isc_result_t
+void
 isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
 
 isc_result_t

diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h
index f0338c5844bb6d948d75fc0dc61990b07e30cd20..0d49b9587cf8a702a6cf6b0e76213394bcc4de80 100644 (file)
--- a/lib/isc/include/isc/util.h
+++ b/lib/isc/include/isc/util.h
@@ -122,8 +122,6 @@
 /*%
  * We use macros instead of calling the routines directly because
  * the capital letters make the locking stand out.
- * We RUNTIME_CHECK for success since in general there's no way
- * for us to continue if they fail.
  */
 
 #ifdef ISC_UTIL_TRACEON
@@ -136,42 +134,40 @@
 #include <isc/result.h> /* Contractual promise. */
 
 #define LOCK(lp)                                                           \
-       do {                                                               \
+       {                                                                  \
                ISC_UTIL_TRACE(fprintf(stderr, "LOCKING %p %s %d\n", (lp), \
                                       __FILE__, __LINE__));               \
-               RUNTIME_CHECK(isc_mutex_lock((lp)) == ISC_R_SUCCESS);      \
+               isc_mutex_lock((lp));                                      \
                ISC_UTIL_TRACE(fprintf(stderr, "LOCKED %p %s %d\n", (lp),  \
                                       __FILE__, __LINE__));               \
-       } while (0)
+       }
 #define UNLOCK(lp)                                                          \
-       do {                                                                \
-               RUNTIME_CHECK(isc_mutex_unlock((lp)) == ISC_R_SUCCESS);     \
+       {                                                                   \
+               isc_mutex_unlock((lp));                                     \
                ISC_UTIL_TRACE(fprintf(stderr, "UNLOCKED %p %s %d\n", (lp), \
                                       __FILE__, __LINE__));                \
-       } while (0)
+       }
 
 #define BROADCAST(cvp)                                                        \
-       do {                                                                  \
+       {                                                                     \
                ISC_UTIL_TRACE(fprintf(stderr, "BROADCAST %p %s %d\n", (cvp), \
                                       __FILE__, __LINE__));                  \
-               RUNTIME_CHECK(isc_condition_broadcast((cvp)) ==               \
-                             ISC_R_SUCCESS);                                 \
-       } while (0)
-#define SIGNAL(cvp)                                                          \
-       do {                                                                 \
-               ISC_UTIL_TRACE(fprintf(stderr, "SIGNAL %p %s %d\n", (cvp),   \
-                                      __FILE__, __LINE__));                 \
-               RUNTIME_CHECK(isc_condition_signal((cvp)) == ISC_R_SUCCESS); \
-       } while (0)
+               isc_condition_broadcast((cvp));                               \
+       }
+#define SIGNAL(cvp)                                                        \
+       {                                                                  \
+               ISC_UTIL_TRACE(fprintf(stderr, "SIGNAL %p %s %d\n", (cvp), \
+                                      __FILE__, __LINE__));               \
+               isc_condition_signal((cvp));                               \
+       }
 #define WAIT(cvp, lp)                                                         \
-       do {                                                                  \
+       {                                                                     \
                ISC_UTIL_TRACE(fprintf(stderr, "WAIT %p LOCK %p %s %d\n",     \
                                       (cvp), (lp), __FILE__, __LINE__));     \
-               RUNTIME_CHECK(isc_condition_wait((cvp), (lp)) ==              \
-                             ISC_R_SUCCESS);                                 \
+               isc_condition_wait((cvp), (lp));                              \
                ISC_UTIL_TRACE(fprintf(stderr, "WAITED %p LOCKED %p %s %d\n", \
                                       (cvp), (lp), __FILE__, __LINE__));     \
-       } while (0)
+       }
 
 /*
  * isc_condition_waituntil can return ISC_R_TIMEDOUT, so we
@@ -183,19 +179,19 @@
 #define WAITUNTIL(cvp, lp, tp) isc_condition_waituntil((cvp), (lp), (tp))
 
 #define RWLOCK(lp, t)                                                         \
-       do {                                                                  \
+       {                                                                     \
                ISC_UTIL_TRACE(fprintf(stderr, "RWLOCK %p, %d %s %d\n", (lp), \
                                       (t), __FILE__, __LINE__));             \
-               RUNTIME_CHECK(isc_rwlock_lock((lp), (t)) == ISC_R_SUCCESS);   \
+               isc_rwlock_lock((lp), (t));                                   \
                ISC_UTIL_TRACE(fprintf(stderr, "RWLOCKED %p, %d %s %d\n",     \
                                       (lp), (t), __FILE__, __LINE__));       \
-       } while (0)
-#define RWUNLOCK(lp, t)                                                       \
-       do {                                                                  \
-               ISC_UTIL_TRACE(fprintf(stderr, "RWUNLOCK %p, %d %s %d\n",     \
-                                      (lp), (t), __FILE__, __LINE__));       \
-               RUNTIME_CHECK(isc_rwlock_unlock((lp), (t)) == ISC_R_SUCCESS); \
-       } while (0)
+       }
+#define RWUNLOCK(lp, t)                                                   \
+       {                                                                 \
+               ISC_UTIL_TRACE(fprintf(stderr, "RWUNLOCK %p, %d %s %d\n", \
+                                      (lp), (t), __FILE__, __LINE__));   \
+               isc_rwlock_unlock((lp), (t));                             \
+       }
 
 /*
  * List Macros.

diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c
index e9b7968efef0ae171bf3c60761dbc0f67df943f2..01b7e91f25aded29429af5c18c305d42d0f50912 100644 (file)
--- a/lib/isc/rwlock.c
+++ b/lib/isc/rwlock.c
@@ -45,19 +45,19 @@ isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota,
        atomic_init(&rwl->downgrade, false);
 }
 
-isc_result_t
+void
 isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        switch (type) {
        case isc_rwlocktype_read:
-               REQUIRE(pthread_rwlock_rdlock(&rwl->rwlock) == 0);
+               RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0);
                break;
        case isc_rwlocktype_write:
                while (true) {
-                       REQUIRE(pthread_rwlock_wrlock(&rwl->rwlock) == 0);
+                       RUNTIME_CHECK(pthread_rwlock_wrlock(&rwl->rwlock) == 0);
                        /* Unlock if in middle of downgrade operation */
                        if (atomic_load_acquire(&rwl->downgrade)) {
-                               REQUIRE(pthread_rwlock_unlock(&rwl->rwlock) ==
-                                       0);
+                               RUNTIME_CHECK(pthread_rwlock_unlock(
+                                                     &rwl->rwlock) == 0);
                                while (atomic_load_acquire(&rwl->downgrade)) {
                                }
                                continue;
@@ -68,7 +68,6 @@ isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        default:
                UNREACHABLE();
        }
-       return (ISC_R_SUCCESS);
 }
 
 isc_result_t
@@ -81,7 +80,7 @@ isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        case isc_rwlocktype_write:
                ret = pthread_rwlock_trywrlock(&rwl->rwlock);
                if ((ret == 0) && atomic_load_acquire(&rwl->downgrade)) {
-                       isc_rwlock_unlock(rwl, type);
+                       RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0);
                        return (ISC_R_LOCKBUSY);
                }
                break;
@@ -101,11 +100,10 @@ isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        }
 }
 
-isc_result_t
+void
 isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        UNUSED(type);
-       REQUIRE(pthread_rwlock_unlock(&rwl->rwlock) == 0);
-       return (ISC_R_SUCCESS);
+       RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0);
 }
 
 isc_result_t
@@ -116,12 +114,9 @@ isc_rwlock_tryupgrade(isc_rwlock_t *rwl) {
 
 void
 isc_rwlock_downgrade(isc_rwlock_t *rwl) {
-       isc_result_t result;
        atomic_store_release(&rwl->downgrade, true);
-       result = isc_rwlock_unlock(rwl, isc_rwlocktype_write);
-       RUNTIME_CHECK(result == ISC_R_SUCCESS);
-       result = isc_rwlock_lock(rwl, isc_rwlocktype_read);
-       RUNTIME_CHECK(result == ISC_R_SUCCESS);
+       RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0);
+       RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0);
        atomic_store_release(&rwl->downgrade, false);
 }
 
@@ -170,7 +165,7 @@ isc_rwlock_destroy(isc_rwlock_t *rwl) {
 #define isc_rwlock_pause()
 #endif /* if defined(_MSC_VER) */
 
-static isc_result_t
+static void
 isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
 
 #ifdef ISC_RWLOCK_TRACE
@@ -238,8 +233,8 @@ isc_rwlock_destroy(isc_rwlock_t *rwl) {
                rwl->readers_waiting == 0);
 
        rwl->magic = 0;
-       (void)isc_condition_destroy(&rwl->readable);
-       (void)isc_condition_destroy(&rwl->writeable);
+       isc_condition_destroy(&rwl->readable);
+       isc_condition_destroy(&rwl->writeable);
        isc_mutex_destroy(&rwl->lock);
 }
 
@@ -308,7 +303,7 @@ isc_rwlock_destroy(isc_rwlock_t *rwl) {
 #define WRITER_ACTIVE 0x1
 #define READER_INCR   0x2
 
-static isc_result_t
+static void
 isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        int32_t cntflag;
 
@@ -423,28 +418,23 @@ isc__rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
 #ifdef ISC_RWLOCK_TRACE
        print_lock("postlock", rwl, type);
 #endif /* ifdef ISC_RWLOCK_TRACE */
-
-       return (ISC_R_SUCCESS);
 }
 
-isc_result_t
+void
 isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        int32_t cnt = 0;
        int32_t spins = atomic_load_acquire(&rwl->spins) * 2 + 10;
        int32_t max_cnt = ISC_MAX(spins, RWLOCK_MAX_ADAPTIVE_COUNT);
-       isc_result_t result = ISC_R_SUCCESS;
 
        do {
                if (cnt++ >= max_cnt) {
-                       result = isc__rwlock_lock(rwl, type);
+                       isc__rwlock_lock(rwl, type);
                        break;
                }
                isc_rwlock_pause();
        } while (isc_rwlock_trylock(rwl, type) != ISC_R_SUCCESS);
 
        atomic_fetch_add_release(&rwl->spins, (cnt - spins) / 8);
-
-       return (result);
 }
 
 isc_result_t
@@ -567,7 +557,7 @@ isc_rwlock_downgrade(isc_rwlock_t *rwl) {
        UNLOCK(&rwl->lock);
 }
 
-isc_result_t
+void
 isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
        int32_t prev_cnt;
 
@@ -638,8 +628,6 @@ isc_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type) {
 #ifdef ISC_RWLOCK_TRACE
        print_lock("postunlock", rwl, type);
 #endif /* ifdef ISC_RWLOCK_TRACE */
-
-       return (ISC_R_SUCCESS);
 }
 
 #endif /* USE_PTHREAD_RWLOCK */

diff --git a/lib/isc/timer.c b/lib/isc/timer.c
index 68984fd5e95663850d02da4204a5b7038c718b76..d05abe99a4c0089bb4b044e1634b7d20363ea570 100644 (file)
--- a/lib/isc/timer.c
+++ b/lib/isc/timer.c
@@ -570,7 +570,7 @@ isc__timermgr_destroy(isc_timermgr_t **managerp) {
        /*
         * Clean up.
         */
-       (void)isc_condition_destroy(&manager->wakeup);
+       isc_condition_destroy(&manager->wakeup);
        isc_mutex_destroy(&manager->lock);
        isc_heap_destroy(&manager->heap);
        manager->magic = 0;

diff --git a/tests/isc/timer_test.c b/tests/isc/timer_test.c
index eeea65137104f3b260b93b2c9be59a9738fc561c..5f8a3d9c4327cf40affceba9a69556a93e979f6a 100644 (file)
--- a/tests/isc/timer_test.c
+++ b/tests/isc/timer_test.c
@@ -74,19 +74,12 @@ _teardown(void **state) {
 
 static void
 test_shutdown(void) {
-       isc_result_t result;
-
        /*
         * Signal shutdown processing complete.
         */
-       result = isc_mutex_lock(&mx);
-       assert_int_equal(result, ISC_R_SUCCESS);
-
-       result = isc_condition_signal(&cv);
-       assert_int_equal(result, ISC_R_SUCCESS);
-
-       result = isc_mutex_unlock(&mx);
-       assert_int_equal(result, ISC_R_SUCCESS);
+       LOCK(&mx);
+       SIGNAL(&cv);
+       UNLOCK(&mx);
 }
 
 static void
@@ -109,9 +102,9 @@ setup_test(isc_timertype_t timertype, isc_interval_t *interval,
        result = isc_task_create(taskmgr, 0, &task, 0);
        assert_int_equal(result, ISC_R_SUCCESS);
 
-       isc_mutex_lock(&lasttime_mx);
+       LOCK(&lasttime_mx);
        result = isc_time_now(&lasttime);
-       isc_mutex_unlock(&lasttime_mx);
+       UNLOCK(&lasttime_mx);
        assert_int_equal(result, ISC_R_SUCCESS);
 
        isc_timer_create(timermgr, task, action, (void *)timertype, &timer);
@@ -122,7 +115,7 @@ setup_test(isc_timertype_t timertype, isc_interval_t *interval,
         * Wait for shutdown processing to complete.
         */
        while (atomic_load(&eventcnt) != nevents) {
-               result = isc_condition_wait(&cv, &mx);
+               WAIT(&cv, &mx);
                assert_int_equal(result, ISC_R_SUCCESS);
        }
 
@@ -132,7 +125,7 @@ setup_test(isc_timertype_t timertype, isc_interval_t *interval,
 
        isc_task_detach(&task);
        isc_mutex_destroy(&mx);
-       (void)isc_condition_destroy(&cv);
+       isc_condition_destroy(&cv);
 }
 
 static void