From ad91937e4d5292eef9c27a663978ee1cf24ddcc4 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 5 Aug 2026 14:43:35 +0200 Subject: [PATCH 1/3] feat(sync): add sentry__cond_wake_all --- src/sentry_sync.h | 18 ++++++++++++ tests/unit/test_sync.c | 66 ++++++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 1 + 3 files changed, 85 insertions(+) diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 66309886ae..27e0422dea 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -135,6 +135,21 @@ WakeConditionVariable_PREVISTA(PCONDITION_VARIABLE_PREVISTA ConditionVariable) ConditionVariable->ContinueEvent, INFINITE, FALSE); } +inline void +WakeAllConditionVariable_PREVISTA( + PCONDITION_VARIABLE_PREVISTA ConditionVariable) +{ + if (!ConditionVariable) { + return; + } + + LONG waiters = InterlockedCompareExchange( + (volatile LONG *)&ConditionVariable->Waiters, 0, 0); + while (waiters-- > 0) { + WakeConditionVariable_PREVISTA(ConditionVariable); + } +} + # endif /* _WIN32_WINNT < 0x0600 */ struct sentry__winmutex_s { @@ -195,6 +210,7 @@ typedef CONDITION_VARIABLE_PREVISTA sentry_cond_t; # define sentry__cond_init(CondVar) \ InitializeConditionVariable_PREVISTA(CondVar) # define sentry__cond_wake WakeConditionVariable_PREVISTA +# define sentry__cond_wake_all WakeAllConditionVariable_PREVISTA # define sentry__cond_wait_timeout(CondVar, Lock, Timeout) \ SleepConditionVariableCS_PREVISTA( \ CondVar, &(Lock)->critical_section, Timeout) @@ -202,6 +218,7 @@ typedef CONDITION_VARIABLE_PREVISTA sentry_cond_t; typedef CONDITION_VARIABLE sentry_cond_t; # define sentry__cond_init(CondVar) InitializeConditionVariable(CondVar) # define sentry__cond_wake WakeConditionVariable +# define sentry__cond_wake_all WakeAllConditionVariable # define sentry__cond_wait_timeout(CondVar, Lock, Timeout) \ SleepConditionVariableCS( \ CondVar, &(Lock)->critical_section, Timeout) @@ -344,6 +361,7 @@ typedef pthread_cond_t sentry_cond_t; } \ } while (0) # define sentry__cond_wake pthread_cond_signal +# define sentry__cond_wake_all pthread_cond_broadcast # define sentry__thread_init(ThreadId) \ memset(ThreadId, 0, sizeof(sentry_threadid_t)) # define sentry__thread_spawn(ThreadId, Func, Data) \ diff --git a/tests/unit/test_sync.c b/tests/unit/test_sync.c index 98c5eeef91..cfc2a03fc9 100644 --- a/tests/unit/test_sync.c +++ b/tests/unit/test_sync.c @@ -581,6 +581,72 @@ SENTRY_TEST(bgworker_delayed_shutdown) sentry__bgworker_decref(bgw); } +#define COND_WAKE_ALL_THREADS 2 + +struct cond_wake_all_state { + sentry_mutex_t mutex; + sentry_cond_t waiting_cond; + sentry_cond_t ready_cond; + volatile long waiting; + volatile long woke; + bool ready; +}; + +SENTRY_THREAD_FN +cond_wake_all_thread(void *data) +{ + struct cond_wake_all_state *state = data; + + sentry__mutex_lock(&state->mutex); + sentry__atomic_fetch_and_add(&state->waiting, 1); + sentry__cond_wake(&state->waiting_cond); + while (!state->ready) { + sentry__cond_wait(&state->ready_cond, &state->mutex); + } + sentry__atomic_fetch_and_add(&state->woke, 1); + sentry__mutex_unlock(&state->mutex); + + return 0; +} + +SENTRY_TEST(cond_wake_all) +{ + struct cond_wake_all_state state = { 0 }; + sentry_threadid_t threads[COND_WAKE_ALL_THREADS]; + + sentry__mutex_init(&state.mutex); + sentry__cond_init(&state.waiting_cond); + sentry__cond_init(&state.ready_cond); + + for (int i = 0; i < COND_WAKE_ALL_THREADS; i++) { + sentry__thread_init(&threads[i]); + TEST_ASSERT( + sentry__thread_spawn(&threads[i], cond_wake_all_thread, &state) + == 0); + } + + sentry__mutex_lock(&state.mutex); + while (sentry__atomic_fetch(&state.waiting) < COND_WAKE_ALL_THREADS) { + sentry__cond_wait(&state.waiting_cond, &state.mutex); + } + state.ready = true; + sentry__cond_wake_all(&state.ready_cond); + sentry__mutex_unlock(&state.mutex); + + for (int i = 0; i < COND_WAKE_ALL_THREADS; i++) { + sentry__thread_join(threads[i]); + } + + TEST_CHECK_INT_EQUAL( + sentry__atomic_fetch(&state.woke), COND_WAKE_ALL_THREADS); + +#ifndef SENTRY_PLATFORM_WINDOWS + pthread_cond_destroy(&state.ready_cond); + pthread_cond_destroy(&state.waiting_cond); +#endif + sentry__mutex_free(&state.mutex); +} + SENTRY_TEST(cond_wait_timeout_overflow) { #if !(defined(SENTRY_PLATFORM_MACOS) \ diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 5f0265802e..e51c7fff36 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -102,6 +102,7 @@ XX(client_report_save_raw_envelope) XX(concurrent_init) XX(concurrent_uninit) XX(cond_wait_timeout_overflow) +XX(cond_wake_all) XX(continuation_no_baggage_uses_sdk_dsc) XX(count_sampled_events) XX(crash_context_handler_path_propagation) From 6aa191a9a7ff178bd78e972196e0ee6b815576e0 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 6 Aug 2026 10:11:42 +0200 Subject: [PATCH 2/3] fix(sync): avoid pre-Vista wake-all timeout deadlock WakeAllConditionVariable_PREVISTA used to snapshot the waiter count and then call WakeConditionVariable_PREVISTA once per waiter. Each single-wake call waits for a waiter-side acknowledgement through ContinueEvent. That handshake can block forever during broadcast if one of the snapshotted waiters times out while another waiter is being signaled. The timed-out waiter can move Waiters past the Target value without setting ContinueEvent, leaving SignalObjectAndWait stuck even though broadcast should never depend on a specific waiter resuming. Release the snapshotted number of semaphore slots directly for wake-all. This keeps the waiter snapshot atomic, leaves the single-wake compatibility path unchanged, and makes broadcast tolerant of waiter timeouts. Any racing timeout may leave a later waiter with a spurious wake, which condition variable callers already have to handle by re-checking their predicate. --- src/sentry_sync.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 27e0422dea..2991d7f64c 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -145,8 +145,8 @@ WakeAllConditionVariable_PREVISTA( LONG waiters = InterlockedCompareExchange( (volatile LONG *)&ConditionVariable->Waiters, 0, 0); - while (waiters-- > 0) { - WakeConditionVariable_PREVISTA(ConditionVariable); + if (waiters > 0) { + ReleaseSemaphore(ConditionVariable->Semaphore, waiters, NULL); } } From 059cb4d7bd4965eb1c5534430e4695e1a1393dae Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 6 Aug 2026 10:39:08 +0200 Subject: [PATCH 3/3] fix(sync): suppress stale pre-Vista broadcast acknowledgements The pre-Vista wake-all path now releases semaphore slots directly instead of calling the blocking single-wake helper for every snapshotted waiter. That avoids the timeout deadlock, but broadcast-woken waiters still run through the shared waiter acknowledgement code in SleepConditionVariableCS_PREVISTA. If Target keeps its previous single-wake value, one of those broadcast waiters can set ContinueEvent after WakeAllConditionVariable_PREVISTA has already returned. The event then remains signaled and can be consumed by a later WakeConditionVariable_PREVISTA call, causing that single wake to return without a fresh waiter acknowledgement. Set Target to an impossible waiter count before releasing the broadcast semaphore slots. This keeps broadcast non-blocking, leaves the single-wake handshake intact, and prevents wake-all from leaking acknowledgements into future single-wake operations. --- src/sentry_sync.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 2991d7f64c..1a9c48a6c6 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -146,6 +146,7 @@ WakeAllConditionVariable_PREVISTA( LONG waiters = InterlockedCompareExchange( (volatile LONG *)&ConditionVariable->Waiters, 0, 0); if (waiters > 0) { + ConditionVariable->Target = -1; ReleaseSemaphore(ConditionVariable->Semaphore, waiters, NULL); } }