We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5538

Summary: [PATCH] Support using CRITICAL_SECTION-based mutexes with condition variables on Windows
Product: SDL Reporter: Cameron Gutman <cameron.gutman>
Component: threadAssignee: Joel Linn <jl>
Status: ASSIGNED --- QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: sezeroz
Version: HG 2.0   
Hardware: x86   
OS: Windows 10   
Attachments: Patch
Patch v2

Description Cameron Gutman 2021-02-09 01:09:19 UTC 
In Vista, Microsoft added both the SleepConditionVariableSRW API that SDL currently uses with the SRW-based mutex implementation and also the SleepConditionVariableCS API which works with CRITICAL_SECTION-based mutexes. We currently only support the former, and this patch adds support for the latter. 

I've also renamed the SDL_syscond_srw.c file and SDL_cond_srw struct now that they no longer require SRW locks.

As a side effect of implementing this support, SDL_HINT_WINDOWS_FORCE_MUTEX_CRITICAL_SECTIONS=1 no longer disables the use of the CONDITION_VARIABLE-based implementation. I can't really see a reason why someone would want to turn off the faster condition variable implementation, but I can add a hint for it if you think that's important.
Comment 1 Cameron Gutman 2021-02-09 01:10:03 UTC 
Created attachment 4783 [details]
Patch
Comment 2 Ozkan Sezer 2021-02-09 13:16:00 UTC 
This will hit -Werror=declaration-after-statement
Comment 3 Joel Linn 2021-02-09 23:27:34 UTC 
With this we have three implementations for condvars... I deliberately ignored the Vista case because of that. Adding another hint would further increase the possible combinations. Thoughts about this from a maintainer?

Will review the diff itself later
Comment 4 Cameron Gutman 2021-02-10 00:25:36 UTC 
Created attachment 4788 [details]
Patch v2

@Ozkan: Thanks for catching that. Fixed it in v2.

@Joel: We still only have 2 implementations of CVs. The only function that needed to change was SDL_CondWaitTimeout() and it was just the addition of a few lines (since the CS implementation is actually a bit simpler than the SRW implementation due to the lack of any internal SDL lock state). We're just calling SleepConditionVariableCS() instead of SleepConditionVariableSRW() if we're using CS-based mutexes.

The majority of the diff is that SDL_syscond_srw -> SDL_syscond_cv name change.