diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index dec427d79..a77716047 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -49,7 +49,11 @@ static HANDLE g_ipc_mutex = NULL; #elif defined(SENTRY_PLATFORM_MACOS) // macOS uses a plain pthread mutex instead of named semaphores (sem_open) // because App Sandbox blocks POSIX named semaphores. +# ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(g_ipc_sync_mutex) +# else static sentry_mutex_t g_ipc_sync_mutex = SENTRY__MUTEX_INIT; +# endif #else # include static sem_t *g_ipc_init_sem = SEM_FAILED; @@ -250,6 +254,7 @@ native_backend_startup( #elif defined(SENTRY_PLATFORM_IOS) state->ipc = sentry__crash_ipc_init_app(NULL); #elif defined(SENTRY_PLATFORM_MACOS) + SENTRY__MUTEX_INIT_DYN_ONCE(g_ipc_sync_mutex); state->ipc = sentry__crash_ipc_init_app(&g_ipc_sync_mutex); #else state->ipc = sentry__crash_ipc_init_app(g_ipc_init_sem); diff --git a/src/modulefinder/sentry_modulefinder_apple.c b/src/modulefinder/sentry_modulefinder_apple.c index b7c8d7266..527f03251 100644 --- a/src/modulefinder/sentry_modulefinder_apple.c +++ b/src/modulefinder/sentry_modulefinder_apple.c @@ -31,7 +31,11 @@ typedef struct segment_command_64 mach_segment_command_type; #endif static bool g_initialized = false; +#ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(g_mutex) +#else static sentry_mutex_t g_mutex = SENTRY__MUTEX_INIT; +#endif static sentry_value_t g_modules = { 0 }; static void @@ -77,6 +81,7 @@ add_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide)) } } + SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex); sentry__mutex_lock(&g_mutex); sentry_value_t modules = g_modules; @@ -98,6 +103,7 @@ remove_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide)) return; } + SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex); sentry__mutex_lock(&g_mutex); if (sentry_value_is_null(g_modules) @@ -135,6 +141,7 @@ sentry_get_modules_list(void) // `add_image` callback). We do that because we have observed deadlocks when // code concurrently `dlopen`s and thus invokes the `add_image` callback // from a different thread. + SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex); sentry__mutex_lock(&g_mutex); if (!g_initialized) { g_modules = sentry_value_new_list(); @@ -162,6 +169,7 @@ sentry_get_modules_list(void) void sentry_clear_modulecache(void) { + SENTRY__MUTEX_INIT_DYN_ONCE(g_mutex); sentry__mutex_lock(&g_mutex); sentry_value_decref(g_modules); g_modules = sentry_value_new_null(); diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index ef5f2d9cd..31bb24445 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -58,7 +58,11 @@ sentry__app_hang_monitor_set_stackwalk_fn(sentry__app_hang_stackwalk_fn fn) static bool g_running = false; static sentry_threadid_t g_thread; +# ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(g_wait_mutex) +# else static sentry_mutex_t g_wait_mutex = SENTRY__MUTEX_INIT; +# endif static sentry_cond_t g_wait_cond; static uint64_t g_timeout_ms = 0; @@ -127,6 +131,7 @@ sentry__app_hang_monitor_start(const sentry_options_t *options) return 0; } + SENTRY__MUTEX_INIT_DYN_ONCE(g_wait_mutex); g_timeout_ms = options->app_hang_timeout; sentry__cond_init(&g_wait_cond); // Arm before spawning: the worker uses is_active() as its run condition, so @@ -150,6 +155,7 @@ sentry__app_hang_monitor_stop(void) return; } sentry__app_hang_set_active(false); + SENTRY__MUTEX_INIT_DYN_ONCE(g_wait_mutex); sentry__mutex_lock(&g_wait_mutex); sentry__cond_wake(&g_wait_cond); sentry__mutex_unlock(&g_wait_mutex); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 00439d689..4dbbb551b 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -26,12 +26,9 @@ #endif static bool g_scope_initialized = false; +static bool g_scope_flush_pending = false; static sentry_scope_t g_scope = { 0 }; -#ifdef SENTRY__MUTEX_INIT_DYN -SENTRY__MUTEX_INIT_DYN(g_lock) -#else -static sentry_mutex_t g_lock = SENTRY__MUTEX_INIT; -#endif +SENTRY__RWLOCK_INIT_DYN(g_lock) static sentry_value_t get_client_sdk(void) @@ -133,43 +130,76 @@ cleanup_scope(sentry_scope_t *scope) void sentry__scope_cleanup(void) { - SENTRY__MUTEX_INIT_DYN_ONCE(g_lock); - sentry__mutex_lock(&g_lock); + SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock); + sentry__rwlock_write_lock(&g_lock); if (g_scope_initialized) { g_scope_initialized = false; + g_scope_flush_pending = false; cleanup_scope(&g_scope); } - sentry__mutex_unlock(&g_lock); + sentry__rwlock_unlock(&g_lock); } -sentry_scope_t * -sentry__scope_lock(void) +const sentry_scope_t * +sentry__scope_read_lock(void) { - SENTRY__MUTEX_INIT_DYN_ONCE(g_lock); - sentry__mutex_lock(&g_lock); - return get_scope(); + SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock); + for (;;) { + sentry__rwlock_read_lock(&g_lock); + if (g_scope_initialized) { + return &g_scope; + } + sentry__rwlock_unlock(&g_lock); + + sentry_scope_t *scope = sentry__scope_write_lock(); + (void)scope; + sentry__scope_write_unlock(); + } } void -sentry__scope_unlock(void) +sentry__scope_read_unlock(void) { - SENTRY__MUTEX_INIT_DYN_ONCE(g_lock); - sentry__mutex_unlock(&g_lock); + SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock); + sentry__rwlock_unlock(&g_lock); } void -sentry__scope_flush_unlock(void) -{ - sentry__scope_unlock(); - SENTRY_WITH_OPTIONS (options) { - // we try to unlock the scope as soon as possible. The - // backend will do its own `WITH_SCOPE` internally. - if (options->backend && options->backend->flush_scope_func) { - options->backend->flush_scope_func(options->backend, options); +sentry__scope_write_unlock(void) +{ + SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock); + bool should_flush + = g_scope_flush_pending && sentry__rwlock_write_depth(&g_lock) == 1; + if (should_flush) { + g_scope_flush_pending = false; + } + bool released_outermost = sentry__rwlock_unlock(&g_lock); + if (released_outermost && should_flush) { + SENTRY_WITH_OPTIONS (options) { + // we try to unlock the scope as soon as possible. The + // backend will do its own `WITH_SCOPE` internally. + if (options->backend && options->backend->flush_scope_func) { + options->backend->flush_scope_func(options->backend, options); + } } } } +sentry_scope_t * +sentry__scope_write_lock(void) +{ + SENTRY__RWLOCK_INIT_DYN_ONCE(g_lock); + sentry__rwlock_write_lock(&g_lock); + return get_scope(); +} + +void +sentry__scope_flush_write_unlock(void) +{ + g_scope_flush_pending = true; + sentry__scope_write_unlock(); +} + sentry_scope_t * sentry_scope_new(void) { diff --git a/src/sentry_scope.h b/src/sentry_scope.h index 86f156a17..81ed26806 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -62,14 +62,24 @@ typedef enum { } sentry_scope_mode_t; /** - * This will acquire a lock on the global scope. + * This will acquire a read lock on the global scope. */ -sentry_scope_t *sentry__scope_lock(void); +const sentry_scope_t *sentry__scope_read_lock(void); /** - * Release the lock on the global scope. + * Release the read lock on the global scope. */ -void sentry__scope_unlock(void); +void sentry__scope_read_unlock(void); + +/** + * This will acquire a write lock on the global scope. + */ +sentry_scope_t *sentry__scope_write_lock(void); + +/** + * Release the write lock on the global scope. + */ +void sentry__scope_write_unlock(void); /** * This will free all the data attached to the global scope @@ -81,7 +91,7 @@ void sentry__scope_cleanup(void); * This function must be called while holding the scope lock, and it will be * unlocked internally. */ -void sentry__scope_flush_unlock(void); +void sentry__scope_flush_write_unlock(void); /** * This will merge the requested data which is in the given `scope` to the given @@ -118,14 +128,14 @@ void sentry__scope_remove_attribute_n( * inside a code block. */ #define SENTRY_WITH_SCOPE(Scope) \ - for (const sentry_scope_t *Scope = sentry__scope_lock(); Scope; \ - sentry__scope_unlock(), Scope = NULL) + for (const sentry_scope_t *Scope = sentry__scope_read_lock(); Scope; \ + sentry__scope_read_unlock(), Scope = NULL) #define SENTRY_WITH_SCOPE_MUT(Scope) \ - for (sentry_scope_t *Scope = sentry__scope_lock(); Scope; \ - sentry__scope_flush_unlock(), Scope = NULL) + for (sentry_scope_t *Scope = sentry__scope_write_lock(); Scope; \ + sentry__scope_flush_write_unlock(), Scope = NULL) #define SENTRY_WITH_SCOPE_MUT_NO_FLUSH(Scope) \ - for (sentry_scope_t *Scope = sentry__scope_lock(); Scope; \ - sentry__scope_unlock(), Scope = NULL) + for (sentry_scope_t *Scope = sentry__scope_write_lock(); Scope; \ + sentry__scope_write_unlock(), Scope = NULL) /** * Rebuilds the scope's dynamic sampling context (DSC) from the SDK options diff --git a/src/sentry_sync.h b/src/sentry_sync.h index adc904fb2..e86a9ccd6 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -284,7 +284,9 @@ typedef pthread_cond_t sentry_cond_t; PTHREAD_MUTEX_RECURSIVE \ } \ } -# elif defined(__FreeBSD__) || defined(SENTRY_PLATFORM_NX) +# elif defined(__FreeBSD__) || defined(SENTRY_PLATFORM_NX) \ + || (defined(SENTRY_PLATFORM_DARWIN) \ + && !defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER)) // Don't define `SENTRY__MUTEX_INIT` but instead provide a new definition that // can be used by platforms requiring dynamic recursive mutex initialization. # define SENTRY__MUTEX_INIT_DYN(Mutex) \ @@ -369,6 +371,193 @@ sentry__cond_wait_timeout( } #endif +typedef struct { + sentry_mutex_t mutex; + sentry_cond_t reader_cond; + sentry_cond_t writer_cond; + size_t readers; +#ifdef SENTRY_PLATFORM_WINDOWS + DWORD writer_owner; +#else + sentry_threadid_t writer_owner; +#endif + size_t writer_depth; + size_t waiting_writers; +} sentry_rwlock_t; + +#ifdef SENTRY_PLATFORM_WINDOWS +# define SENTRY__RWLOCK_INIT_DYN(Lock) \ + static sentry_rwlock_t Lock; \ + static INIT_ONCE Lock##_init_once = INIT_ONCE_STATIC_INIT; \ + static BOOL CALLBACK init_##Lock(PINIT_ONCE UNUSED(InitOnce), \ + PVOID UNUSED(Parameter), PVOID *UNUSED(Context)) \ + { \ + sentry__rwlock_init(&Lock); \ + return TRUE; \ + } +# define SENTRY__RWLOCK_INIT_DYN_ONCE(Lock) \ + InitOnceExecuteOnce(&Lock##_init_once, init_##Lock, NULL, NULL) +#else +# define SENTRY__RWLOCK_INIT_DYN(Lock) \ + static sentry_rwlock_t Lock; \ + static pthread_once_t Lock##_init_once = PTHREAD_ONCE_INIT; \ + static void init_##Lock(void) { sentry__rwlock_init(&Lock); } +# define SENTRY__RWLOCK_INIT_DYN_ONCE(Lock) \ + pthread_once(&Lock##_init_once, init_##Lock) +#endif + +static inline void +sentry__rwlock_init(sentry_rwlock_t *lock) +{ + sentry__mutex_init(&lock->mutex); + sentry__cond_init(&lock->reader_cond); + sentry__cond_init(&lock->writer_cond); + lock->readers = 0; +#ifdef SENTRY_PLATFORM_WINDOWS + lock->writer_owner = 0; +#else + sentry__thread_init(&lock->writer_owner); +#endif + lock->writer_depth = 0; + lock->waiting_writers = 0; +} + +static inline bool +sentry__rwlock_is_writer(sentry_rwlock_t *lock) +{ + if (lock->writer_depth == 0) { + return false; + } +#ifdef SENTRY_PLATFORM_WINDOWS + return lock->writer_owner == GetCurrentThreadId(); +#else + return sentry__threadid_equal(lock->writer_owner, sentry__current_thread()); +#endif +} + +static inline void +sentry__rwlock_set_writer(sentry_rwlock_t *lock) +{ +#ifdef SENTRY_PLATFORM_WINDOWS + lock->writer_owner = GetCurrentThreadId(); +#else + lock->writer_owner = sentry__current_thread(); +#endif +} + +static inline void +sentry__rwlock_read_lock(sentry_rwlock_t *lock) +{ +#ifndef SENTRY_PLATFORM_WINDOWS + if (!sentry__block_for_signal_handler()) { + return; + } +#endif + sentry__mutex_lock(&lock->mutex); + if (sentry__rwlock_is_writer(lock)) { + lock->writer_depth++; + sentry__mutex_unlock(&lock->mutex); + return; + } + while (lock->writer_depth > 0 || lock->waiting_writers > 0) { + sentry__cond_wait(&lock->reader_cond, &lock->mutex); + } + lock->readers++; + sentry__cond_wake(&lock->reader_cond); + sentry__mutex_unlock(&lock->mutex); +} + +static inline bool +sentry__rwlock_release_writer_depth(sentry_rwlock_t *lock) +{ + assert(sentry__rwlock_is_writer(lock)); + lock->writer_depth--; + if (lock->writer_depth > 0) { + return false; + } +#ifdef SENTRY_PLATFORM_WINDOWS + lock->writer_owner = 0; +#else + sentry__thread_init(&lock->writer_owner); +#endif + if (lock->waiting_writers > 0) { + sentry__cond_wake(&lock->writer_cond); + } else { + sentry__cond_wake(&lock->reader_cond); + } + return true; +} + +// Returns true when unlocking releases the outermost write lock. +static inline bool +sentry__rwlock_unlock(sentry_rwlock_t *lock) +{ +#ifndef SENTRY_PLATFORM_WINDOWS + if (!sentry__block_for_signal_handler()) { + return true; + } +#endif + sentry__mutex_lock(&lock->mutex); + bool released_outermost = false; + if (sentry__rwlock_is_writer(lock)) { + released_outermost = sentry__rwlock_release_writer_depth(lock); + sentry__mutex_unlock(&lock->mutex); + return released_outermost; + } + assert(lock->readers > 0); + lock->readers--; + if (lock->readers == 0) { + sentry__cond_wake(&lock->writer_cond); + } + sentry__mutex_unlock(&lock->mutex); + return released_outermost; +} + +static inline void +sentry__rwlock_write_lock(sentry_rwlock_t *lock) +{ +#ifndef SENTRY_PLATFORM_WINDOWS + if (!sentry__block_for_signal_handler()) { + return; + } +#endif + sentry__mutex_lock(&lock->mutex); + if (sentry__rwlock_is_writer(lock)) { + lock->writer_depth++; + sentry__mutex_unlock(&lock->mutex); + return; + } + lock->waiting_writers++; + while (lock->writer_depth > 0 || lock->readers > 0) { + sentry__cond_wait(&lock->writer_cond, &lock->mutex); + } + lock->waiting_writers--; + sentry__rwlock_set_writer(lock); + lock->writer_depth = 1; + sentry__mutex_unlock(&lock->mutex); +} + +static inline size_t +sentry__rwlock_write_depth(sentry_rwlock_t *lock) +{ +#ifndef SENTRY_PLATFORM_WINDOWS + if (!sentry__block_for_signal_handler()) { + return 1; + } +#endif + sentry__mutex_lock(&lock->mutex); + assert(sentry__rwlock_is_writer(lock)); + size_t depth = lock->writer_depth; + sentry__mutex_unlock(&lock->mutex); + return depth; +} + +static inline void +sentry__rwlock_free(sentry_rwlock_t *lock) +{ + sentry__mutex_free(&lock->mutex); +} + static inline long sentry__atomic_fetch_and_add(volatile long *val, long diff) { diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index cd360d511..87ad796a3 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -1214,6 +1214,49 @@ SENTRY_TEST(scope_local_attributes) sentry_close(); } +SENTRY_TEST(scope_nested_write_locks) +{ + SENTRY_TEST_OPTIONS_NEW(options); + sentry_init(options); + + bool read_nested_scope = false; + + SENTRY_WITH_SCOPE_MUT (outer_scope) { + sentry_scope_set_tag(outer_scope, "outer", "yes"); + + SENTRY_WITH_SCOPE_MUT (inner_scope) { + TEST_CHECK(inner_scope == outer_scope); + sentry_scope_set_tag(inner_scope, "inner", "yes"); + + SENTRY_WITH_SCOPE (read_scope) { + TEST_CHECK(read_scope == inner_scope); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string( + sentry_value_get_by_key(read_scope->tags, "outer")), + "yes"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string( + sentry_value_get_by_key(read_scope->tags, "inner")), + "yes"); + read_nested_scope = true; + } + } + } + + TEST_CHECK(read_nested_scope); + + SENTRY_WITH_SCOPE (scope) { + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(sentry_value_get_by_key( + scope->tags, "outer")), + "yes"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(sentry_value_get_by_key( + scope->tags, "inner")), + "yes"); + } + + sentry_close(); +} + SENTRY_TEST(scope_ownership) { // `sentry_local_scope_new` makes a one-shot scope, `sentry_scope_new` does diff --git a/tests/unit/test_sync.c b/tests/unit/test_sync.c index 98c5eeef9..63eb10fc0 100644 --- a/tests/unit/test_sync.c +++ b/tests/unit/test_sync.c @@ -3,6 +3,236 @@ #include "sentry_testsupport.h" #include "sentry_utils.h" +SENTRY_TEST(rwlock_recursion) +{ + sentry_rwlock_t lock; + sentry__rwlock_init(&lock); + + sentry__rwlock_read_lock(&lock); + sentry__rwlock_read_lock(&lock); + sentry__rwlock_unlock(&lock); + sentry__rwlock_unlock(&lock); + + sentry__rwlock_write_lock(&lock); + sentry__rwlock_write_lock(&lock); + TEST_CHECK(!sentry__rwlock_unlock(&lock)); + TEST_CHECK(sentry__rwlock_unlock(&lock)); + + sentry__rwlock_write_lock(&lock); + sentry__rwlock_read_lock(&lock); + sentry__rwlock_unlock(&lock); + TEST_CHECK(sentry__rwlock_unlock(&lock)); + + sentry__rwlock_free(&lock); +} + +struct rwlock_test_state { + sentry_rwlock_t lock; + sentry_mutex_t mutex; + sentry_cond_t cond; + int active_readers; + int max_readers; + int entered_readers; + bool entered; + bool release; + bool writer_entered; + bool release_writer; +}; + +SENTRY_THREAD_FN +rwlock_reader_thread(void *_data) +{ + struct rwlock_test_state *state = _data; + + sentry__rwlock_read_lock(&state->lock); + sentry__mutex_lock(&state->mutex); + state->active_readers++; + state->entered_readers++; + if (state->active_readers > state->max_readers) { + state->max_readers = state->active_readers; + } + sentry__cond_wake(&state->cond); + while (!state->release) { + sentry__cond_wait_timeout(&state->cond, &state->mutex, 100); + } + state->active_readers--; + sentry__mutex_unlock(&state->mutex); + sentry__rwlock_unlock(&state->lock); + +#ifdef SENTRY_PLATFORM_WINDOWS + return 0; +#else + return NULL; +#endif +} + +SENTRY_TEST(rwlock_concurrent_readers) +{ + struct rwlock_test_state state; + memset(&state, 0, sizeof(state)); + sentry__rwlock_init(&state.lock); + sentry__mutex_init(&state.mutex); + sentry__cond_init(&state.cond); + + sentry_threadid_t threads[4]; + for (size_t i = 0; i < 4; i++) { + sentry__thread_init(&threads[i]); + TEST_CHECK_INT_EQUAL( + sentry__thread_spawn(&threads[i], rwlock_reader_thread, &state), 0); + } + + sentry__mutex_lock(&state.mutex); + while (state.entered_readers < 4) { + sentry__cond_wait_timeout(&state.cond, &state.mutex, 1000); + } + TEST_CHECK_INT_EQUAL(state.max_readers, 4); + state.release = true; + sentry__cond_wake(&state.cond); + sentry__mutex_unlock(&state.mutex); + + for (size_t i = 0; i < 4; i++) { + sentry__thread_join(threads[i]); + sentry__thread_free(&threads[i]); + } + + sentry__rwlock_free(&state.lock); + sentry__mutex_free(&state.mutex); +} + +SENTRY_THREAD_FN +rwlock_blocked_reader_thread(void *_data) +{ + struct rwlock_test_state *state = _data; + + sentry__rwlock_read_lock(&state->lock); + sentry__mutex_lock(&state->mutex); + state->entered = true; + sentry__cond_wake(&state->cond); + sentry__mutex_unlock(&state->mutex); + sentry__rwlock_unlock(&state->lock); + +#ifdef SENTRY_PLATFORM_WINDOWS + return 0; +#else + return NULL; +#endif +} + +SENTRY_TEST(rwlock_writer_excludes_readers) +{ + struct rwlock_test_state state; + memset(&state, 0, sizeof(state)); + sentry__rwlock_init(&state.lock); + sentry__mutex_init(&state.mutex); + sentry__cond_init(&state.cond); + + sentry__rwlock_write_lock(&state.lock); + + sentry_threadid_t thread; + sentry__thread_init(&thread); + TEST_CHECK_INT_EQUAL( + sentry__thread_spawn(&thread, rwlock_blocked_reader_thread, &state), 0); + + sleep_ms(100); + sentry__mutex_lock(&state.mutex); + TEST_CHECK(!state.entered); + sentry__mutex_unlock(&state.mutex); + + TEST_CHECK(sentry__rwlock_unlock(&state.lock)); + + sentry__mutex_lock(&state.mutex); + while (!state.entered) { + sentry__cond_wait_timeout(&state.cond, &state.mutex, 1000); + } + sentry__mutex_unlock(&state.mutex); + + sentry__thread_join(thread); + sentry__thread_free(&thread); + + sentry__rwlock_free(&state.lock); + sentry__mutex_free(&state.mutex); +} + +SENTRY_THREAD_FN +rwlock_blocked_writer_thread(void *_data) +{ + struct rwlock_test_state *state = _data; + + sentry__rwlock_write_lock(&state->lock); + sentry__mutex_lock(&state->mutex); + state->writer_entered = true; + sentry__cond_wake(&state->cond); + while (!state->release_writer) { + sentry__cond_wait_timeout(&state->cond, &state->mutex, 100); + } + sentry__mutex_unlock(&state->mutex); + sentry__rwlock_unlock(&state->lock); + +#ifdef SENTRY_PLATFORM_WINDOWS + return 0; +#else + return NULL; +#endif +} + +SENTRY_TEST(rwlock_waiting_writer_excludes_new_readers) +{ + struct rwlock_test_state state; + memset(&state, 0, sizeof(state)); + sentry__rwlock_init(&state.lock); + sentry__mutex_init(&state.mutex); + sentry__cond_init(&state.cond); + + sentry__rwlock_read_lock(&state.lock); + + sentry_threadid_t writer_thread; + sentry__thread_init(&writer_thread); + TEST_CHECK_INT_EQUAL(sentry__thread_spawn(&writer_thread, + rwlock_blocked_writer_thread, &state), + 0); + + bool writer_waiting = false; + for (size_t i = 0; i < 1000 && !writer_waiting; i++) { + sentry__mutex_lock(&state.lock.mutex); + writer_waiting = state.lock.waiting_writers > 0; + sentry__mutex_unlock(&state.lock.mutex); + if (!writer_waiting) { + sleep_ms(1); + } + } + TEST_ASSERT(writer_waiting); + + sentry_threadid_t reader_thread; + sentry__thread_init(&reader_thread); + TEST_CHECK_INT_EQUAL(sentry__thread_spawn(&reader_thread, + rwlock_blocked_reader_thread, &state), + 0); + + sleep_ms(100); + sentry__mutex_lock(&state.mutex); + TEST_CHECK(!state.entered); + sentry__mutex_unlock(&state.mutex); + + sentry__rwlock_unlock(&state.lock); + + sentry__mutex_lock(&state.mutex); + while (!state.writer_entered) { + sentry__cond_wait_timeout(&state.cond, &state.mutex, 1000); + } + TEST_CHECK(!state.entered); + state.release_writer = true; + sentry__cond_wake(&state.cond); + sentry__mutex_unlock(&state.mutex); + + sentry__thread_join(writer_thread); + sentry__thread_free(&writer_thread); + sentry__thread_join(reader_thread); + sentry__thread_free(&reader_thread); + + sentry__rwlock_free(&state.lock); + sentry__mutex_free(&state.mutex); +} + struct task_state { int executed; bool running; diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 5e7a6960d..d70137e51 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -280,6 +280,10 @@ 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_concurrent_readers) +XX(rwlock_recursion) +XX(rwlock_waiting_writer_excludes_new_readers) +XX(rwlock_writer_excludes_readers) XX(sampling_before_send) XX(sampling_decision) XX(sampling_transaction) @@ -295,6 +299,7 @@ XX(scope_fingerprint_n) XX(scope_global_attributes) XX(scope_level) XX(scope_local_attributes) +XX(scope_nested_write_locks) XX(scope_ownership) XX(scope_propagation_context) XX(scope_tags)