pushlock: fix possible sleep on unlocked pushlock

This commit is contained in:
Matheus
2026-07-18 17:35:00 -03:00
parent 6e5e9ecdc5
commit d1b499c43e
+25 -21
View File
@@ -2,10 +2,10 @@
#include <semaphore.h> #include <semaphore.h>
#include <logging.h> #include <logging.h>
#define CAS_ACQUIRE__RELAXED(ptr, saved, new) \ #define CAS_ACQUIRE(ptr, saved, new) \
__atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED) __atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)
#define CAS_RELEASE__RELAXED(ptr, saved, new) \ #define CAS_RELEASE(ptr, saved, new) \
__atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_RELEASE, __ATOMIC_RELAXED) __atomic_compare_exchange_n(ptr, &saved, new, false, __ATOMIC_RELEASE, __ATOMIC_RELAXED)
typedef struct pushlock_wait_block_t { typedef struct pushlock_wait_block_t {
@@ -28,13 +28,13 @@ typedef struct pushlock_wait_block_t {
bool pushlock_try_acquire_exclusive(pushlock_t *pushlock) { bool pushlock_try_acquire_exclusive(pushlock_t *pushlock) {
retry_lock: retry_lock:
pushlock_t saved_value = 0; pushlock_t saved_value = 0;
if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
return true; return true;
if (saved_value & PUSHLOCK_FLAGS_ACQUIRED) if (saved_value & PUSHLOCK_FLAGS_ACQUIRED)
return false; return false;
if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
goto retry_lock; goto retry_lock;
return true; return true;
@@ -43,11 +43,15 @@ retry_lock:
void pushlock_acquire_exclusive(pushlock_t *pushlock) { void pushlock_acquire_exclusive(pushlock_t *pushlock) {
retry_lock: retry_lock:
pushlock_t saved_value = 0; pushlock_t saved_value = 0;
if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
return; return;
if ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) == 0 && CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) == 0) {
if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
goto retry_lock;
return; return;
}
pushlock_wait_block_t wait_block; pushlock_wait_block_t wait_block;
SEMAPHORE_INIT(&wait_block.semaphore, 0); SEMAPHORE_INIT(&wait_block.semaphore, 0);
@@ -60,7 +64,7 @@ retry_lock:
__atomic_store_n(&wait_block.shared_count, PUSHLOCK_GET_SHARED_COUNT(saved_value), __ATOMIC_RELAXED); __atomic_store_n(&wait_block.shared_count, PUSHLOCK_GET_SHARED_COUNT(saved_value), __ATOMIC_RELAXED);
} }
if (CAS_RELEASE__RELAXED(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) if (CAS_RELEASE(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED))
semaphore_wait(&wait_block.semaphore, false); semaphore_wait(&wait_block.semaphore, false);
goto retry_lock; goto retry_lock;
@@ -69,14 +73,14 @@ retry_lock:
bool pushlock_try_acquire_shared(pushlock_t *pushlock) { bool pushlock_try_acquire_shared(pushlock_t *pushlock) {
retry_lock: retry_lock:
pushlock_t saved_value = 0; pushlock_t saved_value = 0;
if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED)))
return true; return true;
if (saved_value & PUSHLOCK_FLAGS_EXCLUSIVE) if (saved_value & PUSHLOCK_FLAGS_EXCLUSIVE)
return false; return false;
if ((saved_value & PUSHLOCK_FLAGS_CONTENDED) == 0) { if ((saved_value & PUSHLOCK_FLAGS_CONTENDED) == 0) {
if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) if (!CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value)))
goto retry_lock; goto retry_lock;
return true; return true;
@@ -86,7 +90,7 @@ retry_lock:
return false; return false;
// try to acquire as exclusive // try to acquire as exclusive
if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
goto retry_lock; goto retry_lock;
return true; return true;
@@ -95,7 +99,7 @@ retry_lock:
void pushlock_acquire_shared(pushlock_t *pushlock) { void pushlock_acquire_shared(pushlock_t *pushlock) {
retry_lock: retry_lock:
pushlock_t saved_value = 0; pushlock_t saved_value = 0;
if (CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED))) if (CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED)))
return; return;
if ((saved_value & PUSHLOCK_FLAGS_EXCLUSIVE) || ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) && (saved_value & PUSHLOCK_FLAGS_CONTENDED))) { if ((saved_value & PUSHLOCK_FLAGS_EXCLUSIVE) || ((saved_value & PUSHLOCK_FLAGS_ACQUIRED) && (saved_value & PUSHLOCK_FLAGS_CONTENDED))) {
@@ -105,27 +109,27 @@ retry_lock:
wait_block.next = PUSHLOCK_GET_POINTER(saved_value); wait_block.next = PUSHLOCK_GET_POINTER(saved_value);
__atomic_store_n(&wait_block.shared_count, 0, __ATOMIC_RELAXED); __atomic_store_n(&wait_block.shared_count, 0, __ATOMIC_RELAXED);
if (CAS_RELEASE__RELAXED(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED)) if (CAS_RELEASE(pushlock, saved_value, (uintptr_t)&wait_block | (saved_value & PUSHLOCK_MASK_FLAGS) | PUSHLOCK_FLAGS_CONTENDED))
semaphore_wait(&wait_block.semaphore, false); semaphore_wait(&wait_block.semaphore, false);
goto retry_lock; goto retry_lock;
} }
if (saved_value & PUSHLOCK_FLAGS_ACQUIRED) { if (saved_value & PUSHLOCK_FLAGS_ACQUIRED) {
if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value))) if (!CAS_ACQUIRE(pushlock, saved_value, PUSHLOCK_INCREMENT_SHARED_COUNT(saved_value)))
goto retry_lock; goto retry_lock;
return; return;
} }
if (!CAS_ACQUIRE__RELAXED(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE)) if (!CAS_ACQUIRE(pushlock, saved_value, saved_value | PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE))
goto retry_lock; goto retry_lock;
} }
// TODO: if we are releasing to a shared waiter, we should wake up every shared waiter // TODO: if we are releasing to a shared waiter, we should wake up every shared waiter
void pushlock_release_exclusive(pushlock_t *pushlock) { void pushlock_release_exclusive(pushlock_t *pushlock) {
pushlock_t saved_value = PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE; pushlock_t saved_value = PUSHLOCK_FLAGS_ACQUIRED | PUSHLOCK_FLAGS_EXCLUSIVE;
if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) if (CAS_RELEASE(pushlock, saved_value, 0))
return; return;
#ifdef DEBUG_LOCKS #ifdef DEBUG_LOCKS
@@ -136,7 +140,7 @@ void pushlock_release_exclusive(pushlock_t *pushlock) {
__atomic_thread_fence(__ATOMIC_ACQUIRE); __atomic_thread_fence(__ATOMIC_ACQUIRE);
pushlock_wait_block_t *wait_block = PUSHLOCK_GET_POINTER(saved_value); pushlock_wait_block_t *wait_block = PUSHLOCK_GET_POINTER(saved_value);
if (wait_block->next == NULL) { if (wait_block->next == NULL) {
if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) { if (CAS_RELEASE(pushlock, saved_value, 0)) {
semaphore_signal(&wait_block->semaphore); semaphore_signal(&wait_block->semaphore);
return; return;
} }
@@ -152,7 +156,7 @@ void pushlock_release_exclusive(pushlock_t *pushlock) {
pushlock_wait_block_t *to_wake = wait_block->next; pushlock_wait_block_t *to_wake = wait_block->next;
wait_block->next = NULL; wait_block->next = NULL;
while (!CAS_RELEASE__RELAXED(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); while (!CAS_RELEASE(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED));
semaphore_signal(&to_wake->semaphore); semaphore_signal(&to_wake->semaphore);
} }
@@ -173,7 +177,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s
__atomic_thread_fence(__ATOMIC_ACQUIRE); __atomic_thread_fence(__ATOMIC_ACQUIRE);
if (penultimate_wait_block == NULL) { if (penultimate_wait_block == NULL) {
if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) { if (CAS_RELEASE(pushlock, saved_value, 0)) {
semaphore_signal(&last_wait_block->semaphore); semaphore_signal(&last_wait_block->semaphore);
return; return;
} }
@@ -187,7 +191,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s
penultimate_wait_block->next = NULL; penultimate_wait_block->next = NULL;
while (!CAS_RELEASE__RELAXED(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED)); while (!CAS_RELEASE(pushlock, saved_value, (saved_value & PUSHLOCK_MASK_POINTER_SHARED_COUNT) | PUSHLOCK_FLAGS_CONTENDED));
semaphore_signal(&last_wait_block->semaphore); semaphore_signal(&last_wait_block->semaphore);
} }
@@ -195,7 +199,7 @@ static void pushlock_release_shared_contended(pushlock_t *pushlock, pushlock_t s
void pushlock_release_shared(pushlock_t *pushlock) { void pushlock_release_shared(pushlock_t *pushlock) {
retry_fast_path: retry_fast_path:
pushlock_t saved_value = PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED); pushlock_t saved_value = PUSHLOCK_INCREMENT_SHARED_COUNT(PUSHLOCK_FLAGS_ACQUIRED);
if (CAS_RELEASE__RELAXED(pushlock, saved_value, 0)) if (CAS_RELEASE(pushlock, saved_value, 0))
return; return;
#ifdef DEBUG_LOCKS #ifdef DEBUG_LOCKS
@@ -213,6 +217,6 @@ retry_fast_path:
return; return;
} }
if (!CAS_RELEASE__RELAXED(pushlock, saved_value, PUSHLOCK_DECREMENT_SHARED_COUNT(saved_value))) if (!CAS_RELEASE(pushlock, saved_value, PUSHLOCK_DECREMENT_SHARED_COUNT(saved_value)))
goto retry_fast_path; goto retry_fast_path;
} }