From 219277af071b9bb3734dfaf99696958fa46d6b43 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 5 Aug 2026 16:15:54 +0200 Subject: [PATCH] feat(sync): add read-write lock primitive Add `sentry_rwlock_t` backed by platform native RW locks. --- src/sentry_sync.h | 201 ++++++++++++++++++++++++++++++ tests/unit/test_sync.c | 270 +++++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 7 ++ 3 files changed, 478 insertions(+) diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 01ac4478b..4a8b89bc8 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -9,6 +9,7 @@ // This is a NOP for platforms that support static mutex initialization. #define SENTRY__MUTEX_INIT_DYN_ONCE(Mutex) ((void)0) +#define SENTRY__RWLOCK_INIT_DYN_ONCE(Rwlock) ((void)0) #ifdef _MSC_VER # define THREAD_FUNCTION_API __stdcall @@ -198,6 +199,137 @@ typedef struct sentry__winmutex_s sentry_mutex_t; # define sentry__mutex_free(Lock) \ DeleteCriticalSection(&(Lock)->critical_section) +# if _WIN32_WINNT >= 0x0600 +typedef SRWLOCK sentry_rwlock_t; +# define SENTRY__RWLOCK_INIT SRWLOCK_INIT +# define sentry__rwlock_init(Lock) InitializeSRWLock(Lock) +# define sentry__rwlock_read_lock(Lock) AcquireSRWLockShared(Lock) +# define sentry__rwlock_read_unlock(Lock) ReleaseSRWLockShared(Lock) +# define sentry__rwlock_write_lock(Lock) AcquireSRWLockExclusive(Lock) +# define sentry__rwlock_write_unlock(Lock) ReleaseSRWLockExclusive(Lock) +# define sentry__rwlock_free(Lock) ((void)0) +# else +typedef struct sentry__winrwlock_s { + INIT_ONCE init_once; + CRITICAL_SECTION lock; + CONDITION_VARIABLE_PREVISTA readers_cond; + CONDITION_VARIABLE_PREVISTA writers_cond; + unsigned readers; + unsigned writers_waiting; + bool writer; +} sentry_rwlock_t; + +# define SENTRY__RWLOCK_INIT \ + { INIT_ONCE_STATIC_INIT, { 0 }, { 0 }, { 0 }, 0, 0, false } + +static inline BOOL CALLBACK +sentry__winrwlock_initonce( + PINIT_ONCE UNUSED(InitOnce), PVOID data, PVOID *UNUSED(lpContext)) +{ + sentry_rwlock_t *rwlock = (sentry_rwlock_t *)data; + InitializeCriticalSection(&rwlock->lock); + InitializeConditionVariable_PREVISTA(&rwlock->readers_cond); + InitializeConditionVariable_PREVISTA(&rwlock->writers_cond); + rwlock->readers = 0; + rwlock->writers_waiting = 0; + rwlock->writer = false; + return TRUE; +} + +static inline void +sentry__winrwlock_ensure_init(sentry_rwlock_t *rwlock) +{ + InitOnceExecuteOnce( + &rwlock->init_once, sentry__winrwlock_initonce, rwlock, NULL); +} + +static inline void +sentry__winrwlock_init(sentry_rwlock_t *rwlock) +{ + sentry_rwlock_t tmp = SENTRY__RWLOCK_INIT; + *rwlock = tmp; + sentry__winrwlock_ensure_init(rwlock); +} + +static inline void +sentry__winrwlock_read_lock(sentry_rwlock_t *rwlock) +{ + sentry__winrwlock_ensure_init(rwlock); + EnterCriticalSection(&rwlock->lock); + while (rwlock->writer || rwlock->writers_waiting > 0) { + SleepConditionVariableCS_PREVISTA( + &rwlock->readers_cond, &rwlock->lock, INFINITE); + } + rwlock->readers++; + if (rwlock->writers_waiting == 0) { + WakeConditionVariable_PREVISTA(&rwlock->readers_cond); + } + LeaveCriticalSection(&rwlock->lock); +} + +static inline void +sentry__winrwlock_read_unlock(sentry_rwlock_t *rwlock) +{ + EnterCriticalSection(&rwlock->lock); + assert(rwlock->readers > 0); + rwlock->readers--; + if (rwlock->readers == 0 && rwlock->writers_waiting > 0) { + WakeConditionVariable_PREVISTA(&rwlock->writers_cond); + } + LeaveCriticalSection(&rwlock->lock); +} + +static inline void +sentry__winrwlock_write_lock(sentry_rwlock_t *rwlock) +{ + sentry__winrwlock_ensure_init(rwlock); + EnterCriticalSection(&rwlock->lock); + rwlock->writers_waiting++; + while (rwlock->writer || rwlock->readers > 0) { + SleepConditionVariableCS_PREVISTA( + &rwlock->writers_cond, &rwlock->lock, INFINITE); + } + rwlock->writers_waiting--; + rwlock->writer = true; + LeaveCriticalSection(&rwlock->lock); +} + +static inline void +sentry__winrwlock_write_unlock(sentry_rwlock_t *rwlock) +{ + EnterCriticalSection(&rwlock->lock); + assert(rwlock->writer); + rwlock->writer = false; + if (rwlock->writers_waiting > 0) { + WakeConditionVariable_PREVISTA(&rwlock->writers_cond); + } else { + WakeConditionVariable_PREVISTA(&rwlock->readers_cond); + } + LeaveCriticalSection(&rwlock->lock); +} + +static inline void +sentry__winrwlock_free(sentry_rwlock_t *rwlock) +{ + sentry__winrwlock_ensure_init(rwlock); + DeleteCriticalSection(&rwlock->lock); + CloseHandle(rwlock->readers_cond.Semaphore); + CloseHandle(rwlock->readers_cond.ContinueEvent); + CloseHandle(rwlock->writers_cond.Semaphore); + CloseHandle(rwlock->writers_cond.ContinueEvent); +} + +# define sentry__rwlock_init(Lock) sentry__winrwlock_init(Lock) +# define sentry__rwlock_read_lock(Lock) sentry__winrwlock_read_lock(Lock) +# define sentry__rwlock_read_unlock(Lock) \ + sentry__winrwlock_read_unlock(Lock) +# define sentry__rwlock_write_lock(Lock) \ + sentry__winrwlock_write_lock(Lock) +# define sentry__rwlock_write_unlock(Lock) \ + sentry__winrwlock_write_unlock(Lock) +# define sentry__rwlock_free(Lock) sentry__winrwlock_free(Lock) +# endif + # define sentry__thread_init(ThreadId) *ThreadId = INVALID_HANDLE_VALUE # define sentry__thread_spawn(ThreadId, Func, Data) \ (*ThreadId = CreateThread(NULL, 0, Func, Data, 0, NULL), \ @@ -278,6 +410,7 @@ void sentry__leave_signal_handler(void); typedef pthread_t sentry_threadid_t; typedef pthread_mutex_t sentry_mutex_t; +typedef pthread_rwlock_t sentry_rwlock_t; typedef pthread_cond_t sentry_cond_t; # ifdef SENTRY_PLATFORM_LINUX @@ -359,6 +492,74 @@ typedef pthread_cond_t sentry_cond_t; } \ } while (0) # define sentry__mutex_free(Lock) pthread_mutex_destroy(Lock) +# ifdef PTHREAD_RWLOCK_INITIALIZER +# define SENTRY__RWLOCK_INIT PTHREAD_RWLOCK_INITIALIZER +# endif + +static inline void +sentry__rwlock_init(sentry_rwlock_t *rwlock) +{ +# ifdef PTHREAD_RWLOCK_INITIALIZER + sentry_rwlock_t tmp = SENTRY__RWLOCK_INIT; + *rwlock = tmp; +# else + int rv = pthread_rwlock_init(rwlock, NULL); + (void)rv; + assert(rv == 0); +# endif +} + +# ifndef PTHREAD_RWLOCK_INITIALIZER +# define SENTRY__RWLOCK_INIT_DYN(Rwlock) \ + static sentry_rwlock_t Rwlock; \ + static pthread_once_t Rwlock##_init_once = PTHREAD_ONCE_INIT; \ + static void init_##Rwlock(void) { sentry__rwlock_init(&Rwlock); } +# undef SENTRY__RWLOCK_INIT_DYN_ONCE +# define SENTRY__RWLOCK_INIT_DYN_ONCE(Rwlock) \ + pthread_once(&Rwlock##_init_once, init_##Rwlock) +# endif + +static inline void +sentry__rwlock_read_lock(sentry_rwlock_t *rwlock) +{ + if (sentry__block_for_signal_handler()) { + int rv = pthread_rwlock_rdlock(rwlock); + (void)rv; + assert(rv == 0); + } +} + +static inline void +sentry__rwlock_read_unlock(sentry_rwlock_t *rwlock) +{ + if (sentry__block_for_signal_handler()) { + pthread_rwlock_unlock(rwlock); + } +} + +static inline void +sentry__rwlock_write_lock(sentry_rwlock_t *rwlock) +{ + if (sentry__block_for_signal_handler()) { + int rv = pthread_rwlock_wrlock(rwlock); + (void)rv; + assert(rv == 0); + } +} + +static inline void +sentry__rwlock_write_unlock(sentry_rwlock_t *rwlock) +{ + if (sentry__block_for_signal_handler()) { + pthread_rwlock_unlock(rwlock); + } +} + +static inline void +sentry__rwlock_free(sentry_rwlock_t *rwlock) +{ + pthread_rwlock_destroy(rwlock); +} # define sentry__cond_init(CondVar) \ do { \ diff --git a/tests/unit/test_sync.c b/tests/unit/test_sync.c index 08f6d6f60..cfbaa4dca 100644 --- a/tests/unit/test_sync.c +++ b/tests/unit/test_sync.c @@ -4,6 +4,276 @@ #include "sentry_utils.h" #include +#define RWLOCK_WAIT_TIMEOUT_MS 2000 + +static bool +wait_for_atomic_value(volatile long *value, long expected) +{ + uint64_t deadline = sentry__monotonic_time() + RWLOCK_WAIT_TIMEOUT_MS; + while (sentry__atomic_fetch(value) != expected) { + if (sentry__monotonic_time() >= deadline) { + return false; + } + sentry__thread_yield(); + sleep_ms(1); + } + return true; +} + +#ifdef SENTRY__RWLOCK_INIT_DYN +SENTRY__RWLOCK_INIT_DYN(static_rwlock) +#else +static sentry_rwlock_t static_rwlock = SENTRY__RWLOCK_INIT; +#endif + +SENTRY_TEST(rwlock_static_init) +{ + SENTRY__RWLOCK_INIT_DYN_ONCE(static_rwlock); + + sentry__rwlock_read_lock(&static_rwlock); + sentry__rwlock_read_unlock(&static_rwlock); + + sentry__rwlock_write_lock(&static_rwlock); + sentry__rwlock_write_unlock(&static_rwlock); +} + +SENTRY_TEST(rwlock_dynamic_init) +{ + sentry_rwlock_t rwlock; + sentry__rwlock_init(&rwlock); + + sentry__rwlock_read_lock(&rwlock); + sentry__rwlock_read_unlock(&rwlock); + + sentry__rwlock_write_lock(&rwlock); + sentry__rwlock_write_unlock(&rwlock); + + sentry__rwlock_free(&rwlock); +} + +typedef struct { + sentry_rwlock_t *rwlock; + volatile long start; + volatile long release; + volatile long entered; + volatile long active_readers; +} rwlock_readers_state_t; + +SENTRY_THREAD_FN +rwlock_reader_thread(void *data) +{ + rwlock_readers_state_t *state = (rwlock_readers_state_t *)data; + while (!sentry__atomic_fetch(&state->start)) { + sentry__thread_yield(); + } + + sentry__rwlock_read_lock(state->rwlock); + sentry__atomic_fetch_and_add(&state->active_readers, 1); + sentry__atomic_fetch_and_add(&state->entered, 1); + while (!sentry__atomic_fetch(&state->release)) { + sentry__thread_yield(); + } + sentry__atomic_fetch_and_add(&state->active_readers, -1); + sentry__rwlock_read_unlock(state->rwlock); + return 0; +} + +SENTRY_TEST(rwlock_shared_readers) +{ + enum { NUM_READERS = 8 }; + + sentry_rwlock_t rwlock; + sentry__rwlock_init(&rwlock); + + rwlock_readers_state_t state = { &rwlock, 0, 0, 0, 0 }; + sentry_threadid_t threads[NUM_READERS]; + for (int i = 0; i < NUM_READERS; i++) { + sentry__thread_init(&threads[i]); + TEST_CHECK_INT_EQUAL( + sentry__thread_spawn(&threads[i], rwlock_reader_thread, &state), 0); + } + + sentry__atomic_store(&state.start, 1); + TEST_CHECK(wait_for_atomic_value(&state.entered, NUM_READERS)); + TEST_CHECK_INT_EQUAL( + sentry__atomic_fetch(&state.active_readers), NUM_READERS); + + sentry__atomic_store(&state.release, 1); + for (int i = 0; i < NUM_READERS; i++) { + sentry__thread_join(threads[i]); + } + + sentry__rwlock_free(&rwlock); +} + +typedef struct { + sentry_rwlock_t *rwlock; + bool write; + volatile long attempting; + volatile long entered; + volatile long release; +} rwlock_waiter_state_t; + +SENTRY_THREAD_FN +rwlock_waiter_thread(void *data) +{ + rwlock_waiter_state_t *state = (rwlock_waiter_state_t *)data; + sentry__atomic_store(&state->attempting, 1); + if (state->write) { + sentry__rwlock_write_lock(state->rwlock); + } else { + sentry__rwlock_read_lock(state->rwlock); + } + sentry__atomic_store(&state->entered, 1); + while (!sentry__atomic_fetch(&state->release)) { + sentry__thread_yield(); + } + if (state->write) { + sentry__rwlock_write_unlock(state->rwlock); + } else { + sentry__rwlock_read_unlock(state->rwlock); + } + return 0; +} + +static void +check_rwlock_blocks_waiter(bool holder_write, bool waiter_write) +{ + sentry_rwlock_t rwlock; + sentry__rwlock_init(&rwlock); + + if (holder_write) { + sentry__rwlock_write_lock(&rwlock); + } else { + sentry__rwlock_read_lock(&rwlock); + } + + rwlock_waiter_state_t state = { &rwlock, waiter_write, 0, 0, 0 }; + sentry_threadid_t thread; + sentry__thread_init(&thread); + TEST_CHECK_INT_EQUAL( + sentry__thread_spawn(&thread, rwlock_waiter_thread, &state), 0); + + TEST_CHECK(wait_for_atomic_value(&state.attempting, 1)); + sleep_ms(100); + TEST_CHECK_INT_EQUAL(sentry__atomic_fetch(&state.entered), 0); + + if (holder_write) { + sentry__rwlock_write_unlock(&rwlock); + } else { + sentry__rwlock_read_unlock(&rwlock); + } + + TEST_CHECK(wait_for_atomic_value(&state.entered, 1)); + sentry__atomic_store(&state.release, 1); + sentry__thread_join(thread); + + sentry__rwlock_free(&rwlock); +} + +SENTRY_TEST(rwlock_read_blocks_writer) +{ + check_rwlock_blocks_waiter(false, true); +} + +SENTRY_TEST(rwlock_write_blocks_reader) +{ + check_rwlock_blocks_waiter(true, false); +} + +SENTRY_TEST(rwlock_write_blocks_writer) +{ + check_rwlock_blocks_waiter(true, true); +} + +typedef struct { + sentry_rwlock_t *rwlock; + volatile long start; + volatile long writers_done; + volatile long failed; + long writers_total; + long counter; +} rwlock_stress_state_t; + +SENTRY_THREAD_FN +rwlock_writer_thread(void *data) +{ + rwlock_stress_state_t *state = (rwlock_stress_state_t *)data; + while (!sentry__atomic_fetch(&state->start)) { + sentry__thread_yield(); + } + + for (int i = 0; i < 1000; i++) { + sentry__rwlock_write_lock(state->rwlock); + state->counter++; + sentry__rwlock_write_unlock(state->rwlock); + if (i % 16 == 0) { + sentry__thread_yield(); + } + } + sentry__atomic_fetch_and_add(&state->writers_done, 1); + return 0; +} + +SENTRY_THREAD_FN +rwlock_stress_reader_thread(void *data) +{ + rwlock_stress_state_t *state = (rwlock_stress_state_t *)data; + long previous = 0; + while (!sentry__atomic_fetch(&state->start)) { + sentry__thread_yield(); + } + + while (sentry__atomic_fetch(&state->writers_done) < state->writers_total) { + sentry__rwlock_read_lock(state->rwlock); + long current = state->counter; + sentry__rwlock_read_unlock(state->rwlock); + if (current < previous) { + sentry__atomic_store(&state->failed, 1); + } + previous = current; + sentry__thread_yield(); + } + return 0; +} + +SENTRY_TEST(rwlock_stress) +{ + enum { NUM_WRITERS = 4, NUM_READERS = 4 }; + + sentry_rwlock_t rwlock; + sentry__rwlock_init(&rwlock); + + rwlock_stress_state_t state = { &rwlock, 0, 0, 0, NUM_WRITERS, 0 }; + sentry_threadid_t writers[NUM_WRITERS]; + sentry_threadid_t readers[NUM_READERS]; + + for (int i = 0; i < NUM_WRITERS; i++) { + sentry__thread_init(&writers[i]); + TEST_CHECK_INT_EQUAL( + sentry__thread_spawn(&writers[i], rwlock_writer_thread, &state), 0); + } + for (int i = 0; i < NUM_READERS; i++) { + sentry__thread_init(&readers[i]); + TEST_CHECK_INT_EQUAL(sentry__thread_spawn(&readers[i], + rwlock_stress_reader_thread, &state), + 0); + } + + sentry__atomic_store(&state.start, 1); + for (int i = 0; i < NUM_WRITERS; i++) { + sentry__thread_join(writers[i]); + } + for (int i = 0; i < NUM_READERS; i++) { + sentry__thread_join(readers[i]); + } + + TEST_CHECK_INT_EQUAL(state.counter, NUM_WRITERS * 1000); + TEST_CHECK_INT_EQUAL(sentry__atomic_fetch(&state.failed), 0); + + sentry__rwlock_free(&rwlock); +} + struct task_state { int executed; bool running; diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index b99a6a85f..b541a7bc5 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -345,6 +345,13 @@ XX(ringbuffer_max_size_null_noop) XX(ringbuffer_max_size_post_init) XX(ringbuffer_to_list_null_value_null) XX(ringbuffer_zero_noop) +XX(rwlock_dynamic_init) +XX(rwlock_read_blocks_writer) +XX(rwlock_shared_readers) +XX(rwlock_static_init) +XX(rwlock_stress) +XX(rwlock_write_blocks_reader) +XX(rwlock_write_blocks_writer) XX(sampling_before_send) XX(sampling_decision) XX(sampling_transaction)