feat(sync): add sentry__cond_wake_all - #1961
Merged
Merged
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1961 +/- ##
==========================================
+ Coverage 75.98% 75.99% +0.01%
==========================================
Files 94 94
Lines 22161 22167 +6
Branches 3936 3938 +2
==========================================
+ Hits 16838 16845 +7
Misses 4437 4437
+ Partials 886 885 -1 🚀 New features to boost your workflow:
|
jpnurmi
force-pushed
the
jpnurmi/feat/cond-wake-all
branch
from
August 6, 2026 07:38
e9a8504 to
ad91937
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ad91937. Configure here.
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.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
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.
JoshuaMoelans
approved these changes
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add an internal condition-variable broadcast helper and use the platform-native broadcast primitives where available.
This is needed by the threadpool work in #1883:
sentry__threadpool_flush()can have multiple concurrent callers waiting on the same condition variable. A single wakeup is not sufficient when the pool drains, because it can release only one waiter and leave other flush callers blocked even thoughpendingalready reached zero.For the pre-Vista Windows fallback, add a bounded
WakeAllConditionVariable_PREVISTAimplementation that snapshots the current waiter count before issuing wakeups, so newly arriving or re-entering waiters cannot extend the broadcast loop indefinitely.Based on:
Similar APIs:
#skip-changelog (internal)