fix: dev: Fix the memory ordering in the adaptive read-write lock

On hardware with a weak memory model, the internal read-write lock could
briefly admit a reader and a writer at the same time, risking sporadic
crashes or incorrect data. The reader/writer handshake now uses
sequentially consistent ordering so the two can no longer overlap.

Closes #6060

Merge branch '6060-rwlock-seq-cst-handshake' into 'main'

See merge request isc-projects/bind9!12162
This commit is contained in:
Ondřej Surý 2026-06-08 17:55:37 +02:00
commit b2ad41d3e7

View file

@ -82,54 +82,74 @@ read_indicator_wait_until_empty(isc_rwlock_t *rwl);
#include <stdio.h>
/*
* The reader/writer handshake below is the store-buffer pattern: each side
* publishes a store (the reader into readers_ingress, the writer into
* writers_lock) and then loads the other side's state. These are independent
* atomic objects, so acquire/release ordering does not order the publish store
* against the later check load, and a reader and a writer could both observe
* the other's slot as free and enter their critical sections at once. The
* operations therefore use memory_order_seq_cst, not acquire/release: the
* single total order over the seq_cst operations is what forbids that overlap.
* Do not weaken these to acquire/release -- doing so reintroduces #6060.
*/
static void
read_indicator_arrive(isc_rwlock_t *rwl) {
(void)atomic_fetch_add_release(&rwl->readers_ingress, 1);
(void)atomic_fetch_add_explicit(&rwl->readers_ingress, 1,
memory_order_seq_cst);
}
static void
read_indicator_depart(isc_rwlock_t *rwl) {
(void)atomic_fetch_add_release(&rwl->readers_egress, 1);
(void)atomic_fetch_add_explicit(&rwl->readers_egress, 1,
memory_order_seq_cst);
}
static bool
read_indicator_isempty(isc_rwlock_t *rwl) {
return atomic_load_acquire(&rwl->readers_egress) ==
atomic_load_acquire(&rwl->readers_ingress);
return atomic_load_explicit(&rwl->readers_egress,
memory_order_seq_cst) ==
atomic_load_explicit(&rwl->readers_ingress,
memory_order_seq_cst);
}
static void
writers_barrier_raise(isc_rwlock_t *rwl) {
(void)atomic_fetch_add_release(&rwl->writers_barrier, 1);
(void)atomic_fetch_add_explicit(&rwl->writers_barrier, 1,
memory_order_seq_cst);
}
static void
writers_barrier_lower(isc_rwlock_t *rwl) {
(void)atomic_fetch_sub_release(&rwl->writers_barrier, 1);
(void)atomic_fetch_sub_explicit(&rwl->writers_barrier, 1,
memory_order_seq_cst);
}
static bool
writers_barrier_israised(isc_rwlock_t *rwl) {
return atomic_load_acquire(&rwl->writers_barrier) > 0;
return atomic_load_explicit(&rwl->writers_barrier,
memory_order_seq_cst) > 0;
}
static bool
writers_lock_islocked(isc_rwlock_t *rwl) {
return atomic_load_acquire(&rwl->writers_lock) == ISC_RWLOCK_LOCKED;
return atomic_load_explicit(&rwl->writers_lock, memory_order_seq_cst) ==
ISC_RWLOCK_LOCKED;
}
static bool
writers_lock_acquire(isc_rwlock_t *rwl) {
return atomic_compare_exchange_weak_acq_rel(
return atomic_compare_exchange_weak_explicit(
&rwl->writers_lock, &(bool){ ISC_RWLOCK_UNLOCKED },
ISC_RWLOCK_LOCKED);
ISC_RWLOCK_LOCKED, memory_order_seq_cst, memory_order_seq_cst);
}
static void
writers_lock_release(isc_rwlock_t *rwl) {
REQUIRE(atomic_compare_exchange_strong_acq_rel(
REQUIRE(atomic_compare_exchange_strong_explicit(
&rwl->writers_lock, &(bool){ ISC_RWLOCK_LOCKED },
ISC_RWLOCK_UNLOCKED));
ISC_RWLOCK_UNLOCKED, memory_order_seq_cst,
memory_order_seq_cst));
}
#define ran_out_of_patience(cnt) (cnt >= RWLOCK_MAX_READER_PATIENCE)
@ -143,6 +163,7 @@ isc_rwlock_rdlock(isc_rwlock_t *rwl) {
while (true) {
read_indicator_arrive(rwl);
if (!writers_lock_islocked(rwl)) {
/* Acquired lock in read-only mode */
break;
@ -169,6 +190,7 @@ isc_rwlock_rdlock(isc_rwlock_t *rwl) {
isc_result_t
isc_rwlock_tryrdlock(isc_rwlock_t *rwl) {
read_indicator_arrive(rwl);
if (writers_lock_islocked(rwl)) {
/* Writer has acquired the lock, release the read lock */
read_indicator_depart(rwl);