Improve reporting for read-write lock errors

Replace direct uses of implementation-specific rwlock functions in
lib/isc/include/isc/rwlock.h with preprocessor macros that use
ERRNO_CHECK(), in order to augment rwlock-related error messages with
file/line/caller information and the error string corresponding to
errno.  Adjust the implementation-specific functions for pthreads-based
rwlocks so that they return any errors encountered to the caller instead
of aborting execution immediately using RUNTIME_CHECK().

To keep code modifications simple, make the non-pthreads-based
implementation-specific rwlock functions always return 0; these
functions continue to handle errors using less verbose run-time
assertions as they do not set errno anyway.

diff --git a/lib/isc/include/isc/rwlock.h b/lib/isc/include/isc/rwlock.h
index b9682e051a64deb4ef500dfaddedad169f1b27ec..b6d5232f52379c656a439457ad9d15cf516ac0a7 100644 (file)
--- a/lib/isc/include/isc/rwlock.h
+++ b/lib/isc/include/isc/rwlock.h
@@ -50,11 +50,11 @@ typedef struct isc_rwlock  isc__rwlock_t;
                *rwl = malloc(sizeof(**rwl));   \
                isc__rwlock_init(*rwl, rq, wq); \
        }
-#define isc_rwlock_lock(rwl, type)    isc___rwlock_lock(*rwl, type)
+#define isc_rwlock_lock(rwl, type)    isc__rwlock_lock(*rwl, type)
 #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(*rwl, type)
-#define isc_rwlock_unlock(rwl, type)  isc___rwlock_unlock(*rwl, type)
+#define isc_rwlock_unlock(rwl, type)  isc__rwlock_unlock(*rwl, type)
 #define isc_rwlock_tryupgrade(rwl)    isc___rwlock_tryupgrade(*rwl)
-#define isc_rwlock_downgrade(rwl)     isc___rwlock_downgrade(*rwl)
+#define isc_rwlock_downgrade(rwl)     isc__rwlock_downgrade(*rwl)
 #define isc_rwlock_destroy(rwl)             \
        {                                   \
                isc___rwlock_destroy(*rwl); \
@@ -67,12 +67,12 @@ typedef struct isc_rwlock isc_rwlock_t;
 typedef struct isc_rwlock isc__rwlock_t;
 
 #define isc_rwlock_init(rwl, rq, wq)  isc__rwlock_init(rwl, rq, wq)
-#define isc_rwlock_lock(rwl, type)    isc___rwlock_lock(rwl, type)
+#define isc_rwlock_lock(rwl, type)    isc__rwlock_lock(rwl, type)
 #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(rwl, type)
-#define isc_rwlock_unlock(rwl, type)  isc___rwlock_unlock(rwl, type)
+#define isc_rwlock_unlock(rwl, type)  isc__rwlock_unlock(rwl, type)
 #define isc_rwlock_tryupgrade(rwl)    isc___rwlock_tryupgrade(rwl)
-#define isc_rwlock_downgrade(rwl)     isc___rwlock_downgrade(rwl)
-#define isc_rwlock_destroy(rwl)              isc___rwlock_destroy(rwl)
+#define isc_rwlock_downgrade(rwl)     isc__rwlock_downgrade(rwl)
+#define isc_rwlock_destroy(rwl)              isc__rwlock_destroy(rwl)
 
 #endif /* ISC_TRACK_PTHREADS_OBJECTS */
 
@@ -118,12 +118,12 @@ typedef struct isc_rwlock isc_rwlock_t;
 typedef struct isc_rwlock isc__rwlock_t;
 
 #define isc_rwlock_init(rwl, rq, wq)  isc__rwlock_init(rwl, rq, wq)
-#define isc_rwlock_lock(rwl, type)    isc___rwlock_lock(rwl, type)
+#define isc_rwlock_lock(rwl, type)    isc__rwlock_lock(rwl, type)
 #define isc_rwlock_trylock(rwl, type) isc___rwlock_trylock(rwl, type)
-#define isc_rwlock_unlock(rwl, type)  isc___rwlock_unlock(rwl, type)
+#define isc_rwlock_unlock(rwl, type)  isc__rwlock_unlock(rwl, type)
 #define isc_rwlock_tryupgrade(rwl)    isc___rwlock_tryupgrade(rwl)
-#define isc_rwlock_downgrade(rwl)     isc___rwlock_downgrade(rwl)
-#define isc_rwlock_destroy(rwl)              isc___rwlock_destroy(rwl)
+#define isc_rwlock_downgrade(rwl)     isc__rwlock_downgrade(rwl)
+#define isc_rwlock_destroy(rwl)              isc__rwlock_destroy(rwl)
 
 #endif /* USE_PTHREAD_RWLOCK */
 
@@ -133,26 +133,50 @@ typedef struct isc_rwlock isc__rwlock_t;
                ERRNO_CHECK(isc___rwlock_init, _ret);      \
        }
 
+#define isc__rwlock_lock(rwl, type)                      \
+       {                                                \
+               int _ret = isc___rwlock_lock(rwl, type); \
+               ERRNO_CHECK(isc___rwlock_lock, _ret);    \
+       }
+
+#define isc__rwlock_unlock(rwl, type)                      \
+       {                                                  \
+               int _ret = isc___rwlock_unlock(rwl, type); \
+               ERRNO_CHECK(isc___rwlock_unlock, _ret);    \
+       }
+
+#define isc__rwlock_downgrade(rwl)                         \
+       {                                                  \
+               int _ret = isc___rwlock_downgrade(rwl);    \
+               ERRNO_CHECK(isc___rwlock_downgrade, _ret); \
+       }
+
+#define isc__rwlock_destroy(rwl)                         \
+       {                                                \
+               int _ret = isc___rwlock_destroy(rwl);    \
+               ERRNO_CHECK(isc___rwlock_destroy, _ret); \
+       }
+
 int
 isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota,
                  unsigned int write_quota);
 
-void
+int
 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);
 
-void
+int
 isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type);
 
 isc_result_t
 isc___rwlock_tryupgrade(isc__rwlock_t *rwl);
 
-void
+int
 isc___rwlock_downgrade(isc__rwlock_t *rwl);
 
-void
+int
 isc___rwlock_destroy(isc__rwlock_t *rwl);
 
 ISC_LANG_ENDDECLS

diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c
index 511c88c4f579b065d4221d085ecacaf25866f0d2..ec3afa74d0e033d7012ee79d741ffe7feecf8b2b 100644 (file)
--- a/lib/isc/rwlock.c
+++ b/lib/isc/rwlock.c
@@ -46,26 +46,32 @@ isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota,
        return (ret);
 }
 
-void
+int
 isc___rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
+       int ret;
+
        switch (type) {
        case isc_rwlocktype_read:
-               RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0);
-               break;
+               return (pthread_rwlock_rdlock(&rwl->rwlock));
        case isc_rwlocktype_write:
                while (true) {
-                       RUNTIME_CHECK(pthread_rwlock_wrlock(&rwl->rwlock) == 0);
+                       ret = pthread_rwlock_wrlock(&rwl->rwlock);
+                       if (ret != 0) {
+                               return (ret);
+                       }
                        /* Unlock if in middle of downgrade operation */
                        if (atomic_load_acquire(&rwl->downgrade)) {
-                               RUNTIME_CHECK(pthread_rwlock_unlock(
-                                                     &rwl->rwlock) == 0);
+                               ret = pthread_rwlock_unlock(&rwl->rwlock);
+                               if (ret != 0) {
+                                       return (ret);
+                               }
                                while (atomic_load_acquire(&rwl->downgrade)) {
                                }
                                continue;
                        }
                        break;
                }
-               break;
+               return (0);
        default:
                UNREACHABLE();
        }
@@ -101,10 +107,10 @@ isc___rwlock_trylock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
        }
 }
 
-void
+int
 isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
        UNUSED(type);
-       RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0);
+       return (pthread_rwlock_unlock(&rwl->rwlock));
 }
 
 isc_result_t
@@ -113,17 +119,30 @@ isc___rwlock_tryupgrade(isc__rwlock_t *rwl) {
        return (ISC_R_LOCKBUSY);
 }
 
-void
+int
 isc___rwlock_downgrade(isc__rwlock_t *rwl) {
+       int ret;
+
        atomic_store_release(&rwl->downgrade, true);
-       RUNTIME_CHECK(pthread_rwlock_unlock(&rwl->rwlock) == 0);
-       RUNTIME_CHECK(pthread_rwlock_rdlock(&rwl->rwlock) == 0);
+
+       ret = pthread_rwlock_unlock(&rwl->rwlock);
+       if (ret != 0) {
+               return (ret);
+       }
+
+       ret = pthread_rwlock_rdlock(&rwl->rwlock);
+       if (ret != 0) {
+               return (ret);
+       }
+
        atomic_store_release(&rwl->downgrade, false);
+
+       return (0);
 }
 
-void
+int
 isc___rwlock_destroy(isc__rwlock_t *rwl) {
-       pthread_rwlock_destroy(&rwl->rwlock);
+       return (pthread_rwlock_destroy(&rwl->rwlock));
 }
 
 #else /* if USE_PTHREAD_RWLOCK */
@@ -223,7 +242,7 @@ isc___rwlock_init(isc__rwlock_t *rwl, unsigned int read_quota,
        return (0);
 }
 
-void
+int
 isc___rwlock_destroy(isc__rwlock_t *rwl) {
        REQUIRE(VALID_RWLOCK(rwl));
 
@@ -236,6 +255,8 @@ isc___rwlock_destroy(isc__rwlock_t *rwl) {
        isc_condition_destroy(&rwl->readable);
        isc_condition_destroy(&rwl->writeable);
        isc_mutex_destroy(&rwl->lock);
+
+       return (0);
 }
 
 /*
@@ -420,7 +441,7 @@ rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
 #endif /* ifdef ISC_RWLOCK_TRACE */
 }
 
-void
+int
 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;
@@ -435,6 +456,8 @@ isc___rwlock_lock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
        } while (isc_rwlock_trylock(rwl, type) != ISC_R_SUCCESS);
 
        atomic_fetch_add_release(&rwl->spins, (cnt - spins) / 8);
+
+       return (0);
 }
 
 isc_result_t
@@ -533,7 +556,7 @@ isc___rwlock_tryupgrade(isc__rwlock_t *rwl) {
        return (ISC_R_SUCCESS);
 }
 
-void
+int
 isc___rwlock_downgrade(isc__rwlock_t *rwl) {
        int32_t prev_readers;
 
@@ -555,9 +578,11 @@ isc___rwlock_downgrade(isc__rwlock_t *rwl) {
                BROADCAST(&rwl->readable);
        }
        UNLOCK(&rwl->lock);
+
+       return (0);
 }
 
-void
+int
 isc___rwlock_unlock(isc__rwlock_t *rwl, isc_rwlocktype_t type) {
        int32_t prev_cnt;
 
@@ -628,6 +653,8 @@ 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 (0);
 }
 
 #endif /* USE_PTHREAD_RWLOCK */