From 365b47caee41f525c858a92796674f154dfd9131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 13 Jul 2022 13:19:32 +0200 Subject: [PATCH 1/4] Add an ERRNO_CHECK() preprocessor macro In a number of situations in pthreads-related code, a common sequence of steps is taken: if the value returned by a library function is not 0, pass errno to strerror_r(), log the string returned by the latter, and immediately abort execution. Add an ERRNO_CHECK() preprocessor macro which takes those exact steps and use it wherever (conveniently) possible. Notes: 1. The "log the return value of strerror_r() and abort" pattern is used in a number of other places that this commit does not touch; only "!= 0" checks followed by isc_error_fatal() calls with non-customized error messages are replaced here. 2. This change temporarily breaks file name & line number reporting for isc__mutex_init() errors, to prevent breaking the build. This issue will be rectified in a subsequent change. --- lib/isc/include/isc/condition.h | 14 +++++--------- lib/isc/include/isc/mutex.h | 4 ++-- lib/isc/include/isc/util.h | 14 +++++++++++++- lib/isc/mutex.c | 9 ++------- lib/isc/thread.c | 24 +++++------------------- 5 files changed, 27 insertions(+), 38 deletions(-) diff --git a/lib/isc/include/isc/condition.h b/lib/isc/include/isc/condition.h index 54e497df0f..ca75f10695 100644 --- a/lib/isc/include/isc/condition.h +++ b/lib/isc/include/isc/condition.h @@ -21,20 +21,16 @@ #include #include #include -#include #include #include +#include typedef pthread_cond_t isc_condition_t; -#define isc_condition_init(cond) \ - if (pthread_cond_init(cond, NULL) != 0) { \ - char isc_condition_strbuf[ISC_STRERRORSIZE]; \ - strerror_r(errno, isc_condition_strbuf, \ - sizeof(isc_condition_strbuf)); \ - isc_error_fatal(__FILE__, __LINE__, \ - "pthread_cond_init failed: %s", \ - isc_condition_strbuf); \ +#define isc_condition_init(cond) \ + { \ + int _ret = pthread_cond_init(cond, NULL); \ + ERRNO_CHECK(pthread_cond_init, _ret); \ } #define isc_condition_wait(cp, mp) \ diff --git a/lib/isc/include/isc/mutex.h b/lib/isc/include/isc/mutex.h index c454dd2b24..8a8562bd8c 100644 --- a/lib/isc/include/isc/mutex.h +++ b/lib/isc/include/isc/mutex.h @@ -26,9 +26,9 @@ ISC_LANG_BEGINDECLS typedef pthread_mutex_t isc_mutex_t; void -isc__mutex_init(isc_mutex_t *mp, const char *file, unsigned int line); +isc__mutex_init(isc_mutex_t *mp); -#define isc_mutex_init(mp) isc__mutex_init((mp), __FILE__, __LINE__) +#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) diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 297cad9d10..f0338c5844 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -309,7 +309,10 @@ mock_assert(const int result, const char *const expression, /* * Errors */ -#include /* Contractual promise. */ +#include /* for errno */ + +#include /* Contractual promise. */ +#include /* for ISC_STRERRORSIZE */ /*% Unexpected Error */ #define UNEXPECTED_ERROR isc_error_unexpected @@ -330,6 +333,15 @@ mock_assert(const int result, const char *const expression, #endif /* UNIT_TESTING */ +/*% Runtime check which logs the error string corresponding to errno */ +#define ERRNO_CHECK(func, ret) \ + if ((ret) != 0) { \ + char _strerrorbuf[ISC_STRERRORSIZE]; \ + strerror_r(errno, _strerrorbuf, sizeof(_strerrorbuf)); \ + isc_error_fatal(__FILE__, __LINE__, "%s() failed in %s(): %s", \ + #func, __func__, _strerrorbuf); \ + } + /*% * Time */ diff --git a/lib/isc/mutex.c b/lib/isc/mutex.c index 0b050fa43d..a73c276193 100644 --- a/lib/isc/mutex.c +++ b/lib/isc/mutex.c @@ -41,7 +41,7 @@ initialize_attr(void) { #endif /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ void -isc__mutex_init(isc_mutex_t *mp, const char *file, unsigned int line) { +isc__mutex_init(isc_mutex_t *mp) { int err; #ifdef HAVE_PTHREAD_MUTEX_ADAPTIVE_NP @@ -53,10 +53,5 @@ isc__mutex_init(isc_mutex_t *mp, const char *file, unsigned int line) { #else /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ err = pthread_mutex_init(mp, NULL); #endif /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ - if (err != 0) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(err, strbuf, sizeof(strbuf)); - isc_error_fatal(file, line, "pthread_mutex_init failed: %s", - strbuf); - } + ERRNO_CHECK(pthread_mutex_init, err); } diff --git a/lib/isc/thread.c b/lib/isc/thread.c index 4c7380cac1..fef4998dab 100644 --- a/lib/isc/thread.c +++ b/lib/isc/thread.c @@ -38,13 +38,6 @@ #define THREAD_MINSTACKSIZE (1024U * 1024) #endif /* ifndef THREAD_MINSTACKSIZE */ -#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) { @@ -65,24 +58,18 @@ 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) { - _FATAL(ret, "pthread_attr_getstacksize()"); - } + ERRNO_CHECK(pthread_attr_getstacksize, ret); if (stacksize < THREAD_MINSTACKSIZE) { ret = pthread_attr_setstacksize(&attr, THREAD_MINSTACKSIZE); - if (ret != 0) { - _FATAL(ret, "pthread_attr_setstacksize()"); - } + ERRNO_CHECK(pthread_attr_setstacksize, ret); } #endif /* if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \ * defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) */ ret = pthread_create(thread, &attr, isc__trampoline_run, trampoline_arg); - if (ret != 0) { - _FATAL(ret, "pthread_create()"); - } + ERRNO_CHECK(pthread_create, ret); pthread_attr_destroy(&attr); @@ -92,9 +79,8 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, 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()"); - } + + ERRNO_CHECK(pthread_join, ret); } void From 5759ace07f70ed32a40d733bacd35e062c28bd3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 13 Jul 2022 13:19:32 +0200 Subject: [PATCH 2/4] Handle pthread_*_init() failures consistently isc_rwlock_init() currently detects pthread_rwlock_init() failures using a REQUIRE() assertion. Use the ERRNO_CHECK() macro for that purpose instead, so that read-write lock initialization failures are handled identically as condition variable (pthread_cond_init()) and mutex (pthread_mutex_init()) initialization failures. --- lib/isc/rwlock.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/isc/rwlock.c b/lib/isc/rwlock.c index 2691a44de4..e9b7968efe 100644 --- a/lib/isc/rwlock.c +++ b/lib/isc/rwlock.c @@ -35,9 +35,13 @@ void isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota, unsigned int write_quota) { + int ret; UNUSED(read_quota); UNUSED(write_quota); - REQUIRE(pthread_rwlock_init(&rwl->rwlock, NULL) == 0); + + ret = pthread_rwlock_init(&rwl->rwlock, NULL); + ERRNO_CHECK(pthread_rwlock_init, ret); + atomic_init(&rwl->downgrade, false); } From 8e5e0fa522d84dfc401708b3d4f6dd47a58327bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 13 Jul 2022 13:19:32 +0200 Subject: [PATCH 3/4] Use library constructor to create default mutex attr once Instead of using isc_once_do() on every isc_mutex_init() call, use the global library constructor to initialize the default mutex attr object (optionally with PTHREAD_MUTEX_ADAPTIVE_NP if supported) just once when the library is loaded. --- lib/isc/Makefile.am | 1 + lib/isc/lib.c | 3 +++ lib/isc/mutex.c | 42 +++++++++++++++++++++++------------------- lib/isc/mutex_p.h | 22 ++++++++++++++++++++++ 4 files changed, 49 insertions(+), 19 deletions(-) create mode 100644 lib/isc/mutex_p.h diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index fb443b5d6e..b61c8c88f2 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -154,6 +154,7 @@ libisc_la_SOURCES = \ mem_p.h \ meminfo.c \ mutex.c \ + mutex_p.h \ mutexblock.c \ net.c \ netaddr.c \ diff --git a/lib/isc/lib.c b/lib/isc/lib.c index 7f6b12c4aa..dd42ea792b 100644 --- a/lib/isc/lib.c +++ b/lib/isc/lib.c @@ -21,6 +21,7 @@ #include "config.h" #include "mem_p.h" +#include "mutex_p.h" #include "os_p.h" #include "tls_p.h" #include "trampoline_p.h" @@ -41,6 +42,7 @@ isc__shutdown(void) ISC_DESTRUCTOR; void isc__initialize(void) { isc__os_initialize(); + isc__mutex_initialize(); isc__mem_initialize(); isc__tls_initialize(); isc__trampoline_initialize(); @@ -52,5 +54,6 @@ isc__shutdown(void) { isc__trampoline_shutdown(); isc__tls_shutdown(); isc__mem_shutdown(); + isc__mutex_shutdown(); isc__os_shutdown(); } diff --git a/lib/isc/mutex.c b/lib/isc/mutex.c index a73c276193..075fc37b32 100644 --- a/lib/isc/mutex.c +++ b/lib/isc/mutex.c @@ -26,32 +26,36 @@ #include #include -#ifdef HAVE_PTHREAD_MUTEX_ADAPTIVE_NP -static bool attr_initialized = false; -static pthread_mutexattr_t attr; -static isc_once_t once_attr = ISC_ONCE_INIT; +#include "mutex_p.h" + +pthread_mutexattr_t isc__mutex_init_attr; +static isc_once_t init_once = ISC_ONCE_INIT; static void -initialize_attr(void) { - RUNTIME_CHECK(pthread_mutexattr_init(&attr) == 0); - RUNTIME_CHECK(pthread_mutexattr_settype( - &attr, PTHREAD_MUTEX_ADAPTIVE_NP) == 0); - attr_initialized = true; -} +mutex_initialize(void) { + RUNTIME_CHECK(pthread_mutexattr_init(&isc__mutex_init_attr) == 0); +#ifdef HAVE_PTHREAD_MUTEX_ADAPTIVE_NP + RUNTIME_CHECK(pthread_mutexattr_settype(&isc__mutex_init_attr, + PTHREAD_MUTEX_ADAPTIVE_NP) == + 0); #endif /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ +} + +void +isc__mutex_initialize(void) { + RUNTIME_CHECK(isc_once_do(&init_once, mutex_initialize) == + ISC_R_SUCCESS); +} void isc__mutex_init(isc_mutex_t *mp) { int err; -#ifdef HAVE_PTHREAD_MUTEX_ADAPTIVE_NP - isc_result_t result = ISC_R_SUCCESS; - result = isc_once_do(&once_attr, initialize_attr); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - - err = pthread_mutex_init(mp, &attr); -#else /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ - err = pthread_mutex_init(mp, NULL); -#endif /* HAVE_PTHREAD_MUTEX_ADAPTIVE_NP */ + err = pthread_mutex_init(mp, &isc__mutex_init_attr); ERRNO_CHECK(pthread_mutex_init, err); } + +void +isc__mutex_shutdown(void) { + /* noop */; +} diff --git a/lib/isc/mutex_p.h b/lib/isc/mutex_p.h new file mode 100644 index 0000000000..e9aa8a891b --- /dev/null +++ b/lib/isc/mutex_p.h @@ -0,0 +1,22 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +/*! \file */ + +void +isc__mutex_initialize(void); + +void +isc__mutex_shutdown(void); From deae9743665f594e9eb878e3940216861df43400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 13 Jul 2022 13:19:32 +0200 Subject: [PATCH 4/4] 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. --- bin/dig/dighost.c | 20 +++++------- lib/isc/include/isc/condition.h | 13 +++----- lib/isc/include/isc/mutex.h | 6 ++-- lib/isc/include/isc/rwlock.h | 4 +-- lib/isc/include/isc/util.h | 58 +++++++++++++++------------------ lib/isc/rwlock.c | 46 ++++++++++---------------- lib/isc/timer.c | 2 +- tests/isc/timer_test.c | 21 ++++-------- 8 files changed, 70 insertions(+), 100 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 9b5e1c7309..8ef933b16d 100644 --- 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 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__); \ - check_result(isc_mutex_unlock((&lookup_lock)), "isc_mutex_" \ - "unlock"); \ +#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 ca75f10695..69622d32ce 100644 --- 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 8a8562bd8c..09d192f610 100644 --- 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 a0d7083ca4..ba0ba0d43d 100644 --- 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 f0338c5844..0d49b9587c 100644 --- 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 /* 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 e9b7968efe..01b7e91f25 100644 --- 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 68984fd5e9..d05abe99a4 100644 --- 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 eeea651371..5f8a3d9c43 100644 --- 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