2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-04 16:45:24 +00:00

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.
This commit is contained in:
Ondřej Surý
2022-07-13 13:19:32 +02:00
committed by Michał Kępień
parent 8e5e0fa522
commit deae974366
8 changed files with 70 additions and 100 deletions

View File

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

View File

@@ -33,18 +33,15 @@ typedef pthread_cond_t isc_condition_t;
ERRNO_CHECK(pthread_cond_init, _ret); \ ERRNO_CHECK(pthread_cond_init, _ret); \
} }
#define isc_condition_wait(cp, mp) \ #define isc_condition_wait(cp, mp) \
((pthread_cond_wait((cp), (mp)) == 0) ? ISC_R_SUCCESS \ RUNTIME_CHECK(pthread_cond_wait((cp), (mp)) == 0)
: ISC_R_UNEXPECTED)
#define isc_condition_signal(cp) \ #define isc_condition_signal(cp) RUNTIME_CHECK(pthread_cond_signal((cp)) == 0)
((pthread_cond_signal((cp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
#define isc_condition_broadcast(cp) \ #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) \ #define isc_condition_destroy(cp) RUNTIME_CHECK(pthread_cond_destroy((cp)) == 0)
((pthread_cond_destroy((cp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
ISC_LANG_BEGINDECLS ISC_LANG_BEGINDECLS

View File

@@ -30,11 +30,9 @@ isc__mutex_init(isc_mutex_t *mp);
#define isc_mutex_init(mp) isc__mutex_init((mp)) #define isc_mutex_init(mp) isc__mutex_init((mp))
#define isc_mutex_lock(mp) \ #define isc_mutex_lock(mp) RUNTIME_CHECK(pthread_mutex_lock((mp)) == 0)
((pthread_mutex_lock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
#define isc_mutex_unlock(mp) \ #define isc_mutex_unlock(mp) RUNTIME_CHECK(pthread_mutex_unlock((mp)) == 0)
((pthread_mutex_unlock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_UNEXPECTED)
#define isc_mutex_trylock(mp) \ #define isc_mutex_trylock(mp) \
((pthread_mutex_trylock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_LOCKBUSY) ((pthread_mutex_trylock((mp)) == 0) ? ISC_R_SUCCESS : ISC_R_LOCKBUSY)

View File

@@ -82,13 +82,13 @@ void
isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota, isc_rwlock_init(isc_rwlock_t *rwl, unsigned int read_quota,
unsigned int write_quota); unsigned int write_quota);
isc_result_t void
isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type); isc_rwlock_lock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
isc_result_t isc_result_t
isc_rwlock_trylock(isc_rwlock_t *rwl, isc_rwlocktype_t type); 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_rwlock_unlock(isc_rwlock_t *rwl, isc_rwlocktype_t type);
isc_result_t isc_result_t

View File

@@ -122,8 +122,6 @@
/*% /*%
* We use macros instead of calling the routines directly because * We use macros instead of calling the routines directly because
* the capital letters make the locking stand out. * 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 #ifdef ISC_UTIL_TRACEON
@@ -136,42 +134,40 @@
#include <isc/result.h> /* Contractual promise. */ #include <isc/result.h> /* Contractual promise. */
#define LOCK(lp) \ #define LOCK(lp) \
do { \ { \
ISC_UTIL_TRACE(fprintf(stderr, "LOCKING %p %s %d\n", (lp), \ ISC_UTIL_TRACE(fprintf(stderr, "LOCKING %p %s %d\n", (lp), \
__FILE__, __LINE__)); \ __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), \ ISC_UTIL_TRACE(fprintf(stderr, "LOCKED %p %s %d\n", (lp), \
__FILE__, __LINE__)); \ __FILE__, __LINE__)); \
} while (0) }
#define UNLOCK(lp) \ #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), \ ISC_UTIL_TRACE(fprintf(stderr, "UNLOCKED %p %s %d\n", (lp), \
__FILE__, __LINE__)); \ __FILE__, __LINE__)); \
} while (0) }
#define BROADCAST(cvp) \ #define BROADCAST(cvp) \
do { \ { \
ISC_UTIL_TRACE(fprintf(stderr, "BROADCAST %p %s %d\n", (cvp), \ ISC_UTIL_TRACE(fprintf(stderr, "BROADCAST %p %s %d\n", (cvp), \
__FILE__, __LINE__)); \ __FILE__, __LINE__)); \
RUNTIME_CHECK(isc_condition_broadcast((cvp)) == \ isc_condition_broadcast((cvp)); \
ISC_R_SUCCESS); \ }
} while (0) #define SIGNAL(cvp) \
#define SIGNAL(cvp) \ { \
do { \ ISC_UTIL_TRACE(fprintf(stderr, "SIGNAL %p %s %d\n", (cvp), \
ISC_UTIL_TRACE(fprintf(stderr, "SIGNAL %p %s %d\n", (cvp), \ __FILE__, __LINE__)); \
__FILE__, __LINE__)); \ isc_condition_signal((cvp)); \
RUNTIME_CHECK(isc_condition_signal((cvp)) == ISC_R_SUCCESS); \ }
} while (0)
#define WAIT(cvp, lp) \ #define WAIT(cvp, lp) \
do { \ { \
ISC_UTIL_TRACE(fprintf(stderr, "WAIT %p LOCK %p %s %d\n", \ ISC_UTIL_TRACE(fprintf(stderr, "WAIT %p LOCK %p %s %d\n", \
(cvp), (lp), __FILE__, __LINE__)); \ (cvp), (lp), __FILE__, __LINE__)); \
RUNTIME_CHECK(isc_condition_wait((cvp), (lp)) == \ isc_condition_wait((cvp), (lp)); \
ISC_R_SUCCESS); \
ISC_UTIL_TRACE(fprintf(stderr, "WAITED %p LOCKED %p %s %d\n", \ ISC_UTIL_TRACE(fprintf(stderr, "WAITED %p LOCKED %p %s %d\n", \
(cvp), (lp), __FILE__, __LINE__)); \ (cvp), (lp), __FILE__, __LINE__)); \
} while (0) }
/* /*
* isc_condition_waituntil can return ISC_R_TIMEDOUT, so we * 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 WAITUNTIL(cvp, lp, tp) isc_condition_waituntil((cvp), (lp), (tp))
#define RWLOCK(lp, t) \ #define RWLOCK(lp, t) \
do { \ { \
ISC_UTIL_TRACE(fprintf(stderr, "RWLOCK %p, %d %s %d\n", (lp), \ ISC_UTIL_TRACE(fprintf(stderr, "RWLOCK %p, %d %s %d\n", (lp), \
(t), __FILE__, __LINE__)); \ (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", \ ISC_UTIL_TRACE(fprintf(stderr, "RWLOCKED %p, %d %s %d\n", \
(lp), (t), __FILE__, __LINE__)); \ (lp), (t), __FILE__, __LINE__)); \
} while (0) }
#define RWUNLOCK(lp, t) \ #define RWUNLOCK(lp, t) \
do { \ { \
ISC_UTIL_TRACE(fprintf(stderr, "RWUNLOCK %p, %d %s %d\n", \ ISC_UTIL_TRACE(fprintf(stderr, "RWUNLOCK %p, %d %s %d\n", \
(lp), (t), __FILE__, __LINE__)); \ (lp), (t), __FILE__, __LINE__)); \
RUNTIME_CHECK(isc_rwlock_unlock((lp), (t)) == ISC_R_SUCCESS); \ isc_rwlock_unlock((lp), (t)); \
} while (0) }
/* /*
* List Macros. * List Macros.

View File

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

View File

@@ -570,7 +570,7 @@ isc__timermgr_destroy(isc_timermgr_t **managerp) {
/* /*
* Clean up. * Clean up.
*/ */
(void)isc_condition_destroy(&manager->wakeup); isc_condition_destroy(&manager->wakeup);
isc_mutex_destroy(&manager->lock); isc_mutex_destroy(&manager->lock);
isc_heap_destroy(&manager->heap); isc_heap_destroy(&manager->heap);
manager->magic = 0; manager->magic = 0;

View File

@@ -74,19 +74,12 @@ _teardown(void **state) {
static void static void
test_shutdown(void) { test_shutdown(void) {
isc_result_t result;
/* /*
* Signal shutdown processing complete. * Signal shutdown processing complete.
*/ */
result = isc_mutex_lock(&mx); LOCK(&mx);
assert_int_equal(result, ISC_R_SUCCESS); SIGNAL(&cv);
UNLOCK(&mx);
result = isc_condition_signal(&cv);
assert_int_equal(result, ISC_R_SUCCESS);
result = isc_mutex_unlock(&mx);
assert_int_equal(result, ISC_R_SUCCESS);
} }
static void static void
@@ -109,9 +102,9 @@ setup_test(isc_timertype_t timertype, isc_interval_t *interval,
result = isc_task_create(taskmgr, 0, &task, 0); result = isc_task_create(taskmgr, 0, &task, 0);
assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(result, ISC_R_SUCCESS);
isc_mutex_lock(&lasttime_mx); LOCK(&lasttime_mx);
result = isc_time_now(&lasttime); result = isc_time_now(&lasttime);
isc_mutex_unlock(&lasttime_mx); UNLOCK(&lasttime_mx);
assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(result, ISC_R_SUCCESS);
isc_timer_create(timermgr, task, action, (void *)timertype, &timer); 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. * Wait for shutdown processing to complete.
*/ */
while (atomic_load(&eventcnt) != nevents) { while (atomic_load(&eventcnt) != nevents) {
result = isc_condition_wait(&cv, &mx); WAIT(&cv, &mx);
assert_int_equal(result, ISC_R_SUCCESS); 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_task_detach(&task);
isc_mutex_destroy(&mx); isc_mutex_destroy(&mx);
(void)isc_condition_destroy(&cv); isc_condition_destroy(&cv);
} }
static void static void