From 5dccffd7e4fd78e96a2f20e460ec78d46642f9f9 Mon Sep 17 00:00:00 2001
From: Cameron Gutman <[EMAIL REDACTED]>
Date: Mon, 23 Aug 2021 21:16:58 -0400
Subject: [PATCH] Allow usage of the new Condition Variable code with Critical
Sections
Vista and later provide the SleepConditionVariableCS() function for this.
Since SDL_syscond_srw.c doesn't require SRW locks anymore, rename it to
SDL_syscond_cv.c which better reflects the implementation of condition
variables rather than the implementation of mutexes.
Fixes #4051.
---
CMakeLists.txt | 2 +-
VisualC/SDL/SDL.vcxproj | 2 +-
VisualC/SDL/SDL.vcxproj.filters | 2 +-
include/SDL_hints.h | 3 -
.../{SDL_syscond_srw.c => SDL_syscond_cv.c} | 117 ++++++++++--------
src/thread/windows/SDL_sysmutex.c | 5 -
src/thread/windows/SDL_sysmutex_c.h | 5 +
7 files changed, 75 insertions(+), 61 deletions(-)
rename src/thread/windows/{SDL_syscond_srw.c => SDL_syscond_cv.c} (68%)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 880415ed3b..0a22a556b4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1596,7 +1596,7 @@ elseif(WINDOWS)
set(SDL_THREAD_WINDOWS 1)
set(SOURCE_FILES ${SOURCE_FILES}
${SDL2_SOURCE_DIR}/src/thread/generic/SDL_syscond.c
- ${SDL2_SOURCE_DIR}/src/thread/windows/SDL_syscond_srw.c
+ ${SDL2_SOURCE_DIR}/src/thread/windows/SDL_syscond_cv.c
${SDL2_SOURCE_DIR}/src/thread/windows/SDL_sysmutex.c
${SDL2_SOURCE_DIR}/src/thread/windows/SDL_syssem.c
${SDL2_SOURCE_DIR}/src/thread/windows/SDL_systhread.c
diff --git a/VisualC/SDL/SDL.vcxproj b/VisualC/SDL/SDL.vcxproj
index d5deb7ebe3..5cb8b03884 100644
--- a/VisualC/SDL/SDL.vcxproj
+++ b/VisualC/SDL/SDL.vcxproj
@@ -559,7 +559,7 @@
<ClCompile Include="..\..\src\stdlib\SDL_strtokr.c" />
<ClCompile Include="..\..\src\thread\generic\SDL_syscond.c" />
<ClCompile Include="..\..\src\thread\SDL_thread.c" />
- <ClCompile Include="..\..\src\thread\windows\SDL_syscond_srw.c" />
+ <ClCompile Include="..\..\src\thread\windows\SDL_syscond_cv.c" />
<ClCompile Include="..\..\src\thread\windows\SDL_sysmutex.c" />
<ClCompile Include="..\..\src\thread\windows\SDL_syssem.c" />
<ClCompile Include="..\..\src\thread\windows\SDL_systhread.c" />
diff --git a/VisualC/SDL/SDL.vcxproj.filters b/VisualC/SDL/SDL.vcxproj.filters
index 8fa5c689b8..4924e6cce7 100644
--- a/VisualC/SDL/SDL.vcxproj.filters
+++ b/VisualC/SDL/SDL.vcxproj.filters
@@ -1201,7 +1201,7 @@
<ClCompile Include="..\..\src\thread\SDL_thread.c">
<Filter>thread</Filter>
</ClCompile>
- <ClCompile Include="..\..\src\thread\windows\SDL_syscond_srw.c">
+ <ClCompile Include="..\..\src\thread\windows\SDL_syscond_cv.c" />
<Filter>thread\windows</Filter>
</ClCompile>
<ClCompile Include="..\..\src\thread\windows\SDL_sysmutex.c">
diff --git a/include/SDL_hints.h b/include/SDL_hints.h
index 2c4818c4c1..fdfb7eee86 100644
--- a/include/SDL_hints.h
+++ b/include/SDL_hints.h
@@ -1547,9 +1547,6 @@ extern "C" {
* They offer better performance, allocate no kernel ressources and
* use less memory. SDL will fall back to Critical Sections on older
* OS versions or if forced to by this hint.
- * This also affects Condition Variables. When SRW mutexes are used,
- * SDL will use Windows Condition Variables as well. Else, a generic
- * SDL_cond implementation will be used that works with all mutexes.
*
* This variable can be set to the following values:
* "0" - Use SRW Locks when available. If not, fall back to Critical Sections. (default)
diff --git a/src/thread/windows/SDL_syscond_srw.c b/src/thread/windows/SDL_syscond_cv.c
similarity index 68%
rename from src/thread/windows/SDL_syscond_srw.c
rename to src/thread/windows/SDL_syscond_cv.c
index 763a5465e0..587a585bde 100644
--- a/src/thread/windows/SDL_syscond_srw.c
+++ b/src/thread/windows/SDL_syscond_cv.c
@@ -62,29 +62,32 @@ typedef struct CONDITION_VARIABLE {
#define pWakeConditionVariable WakeConditionVariable
#define pWakeAllConditionVariable WakeAllConditionVariable
#define pSleepConditionVariableSRW SleepConditionVariableSRW
+#define pSleepConditionVariableCS SleepConditionVariableCS
#else
typedef VOID(WINAPI *pfnWakeConditionVariable)(PCONDITION_VARIABLE);
typedef VOID(WINAPI *pfnWakeAllConditionVariable)(PCONDITION_VARIABLE);
typedef BOOL(WINAPI *pfnSleepConditionVariableSRW)(PCONDITION_VARIABLE, PSRWLOCK, DWORD, ULONG);
+typedef BOOL(WINAPI* pfnSleepConditionVariableCS)(PCONDITION_VARIABLE, PCRITICAL_SECTION, DWORD);
static pfnWakeConditionVariable pWakeConditionVariable = NULL;
static pfnWakeAllConditionVariable pWakeAllConditionVariable = NULL;
static pfnSleepConditionVariableSRW pSleepConditionVariableSRW = NULL;
+static pfnSleepConditionVariableCS pSleepConditionVariableCS = NULL;
#endif
-typedef struct SDL_cond_srw
+typedef struct SDL_cond_cv
{
CONDITION_VARIABLE cond;
-} SDL_cond_srw;
+} SDL_cond_cv;
static SDL_cond *
-SDL_CreateCond_srw(void)
+SDL_CreateCond_cv(void)
{
- SDL_cond_srw *cond;
+ SDL_cond_cv *cond;
/* Relies on CONDITION_VARIABLE_INIT == 0. */
- cond = (SDL_cond_srw *) SDL_calloc(1, sizeof(*cond));
+ cond = (SDL_cond_cv *) SDL_calloc(1, sizeof(*cond));
if (!cond) {
SDL_OutOfMemory();
}
@@ -93,7 +96,7 @@ SDL_CreateCond_srw(void)
}
static void
-SDL_DestroyCond_srw(SDL_cond * cond)
+SDL_DestroyCond_cv(SDL_cond * cond)
{
if (cond) {
/* There are no kernel allocated resources */
@@ -102,9 +105,9 @@ SDL_DestroyCond_srw(SDL_cond * cond)
}
static int
-SDL_CondSignal_srw(SDL_cond * _cond)
+SDL_CondSignal_cv(SDL_cond * _cond)
{
- SDL_cond_srw *cond = (SDL_cond_srw *)_cond;
+ SDL_cond_cv *cond = (SDL_cond_cv *)_cond;
if (!cond) {
return SDL_SetError("Passed a NULL condition variable");
}
@@ -115,9 +118,9 @@ SDL_CondSignal_srw(SDL_cond * _cond)
}
static int
-SDL_CondBroadcast_srw(SDL_cond * _cond)
+SDL_CondBroadcast_cv(SDL_cond * _cond)
{
- SDL_cond_srw *cond = (SDL_cond_srw *)_cond;
+ SDL_cond_cv *cond = (SDL_cond_cv *)_cond;
if (!cond) {
return SDL_SetError("Passed a NULL condition variable");
}
@@ -128,65 +131,82 @@ SDL_CondBroadcast_srw(SDL_cond * _cond)
}
static int
-SDL_CondWaitTimeout_srw(SDL_cond * _cond, SDL_mutex * _mutex, Uint32 ms)
+SDL_CondWaitTimeout_cv(SDL_cond * _cond, SDL_mutex * _mutex, Uint32 ms)
{
- SDL_cond_srw *cond = (SDL_cond_srw *)_cond;
- SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex;
+ SDL_cond_cv *cond = (SDL_cond_cv *)_cond;
DWORD timeout;
int ret;
if (!cond) {
return SDL_SetError("Passed a NULL condition variable");
}
- if (!mutex) {
+ if (!_mutex) {
return SDL_SetError("Passed a NULL mutex");
}
- if (mutex->count != 1 || mutex->owner != GetCurrentThreadId()) {
- return SDL_SetError("Passed mutex is not locked or locked recursively");
- }
-
if (ms == SDL_MUTEX_MAXWAIT) {
timeout = INFINITE;
} else {
timeout = (DWORD) ms;
}
- /* The mutex must be updated to the released state */
- mutex->count = 0;
- mutex->owner = 0;
+ if (SDL_mutex_impl_active.Type == SDL_MUTEX_SRW) {
+ SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex;
+
+ if (mutex->count != 1 || mutex->owner != GetCurrentThreadId()) {
+ return SDL_SetError("Passed mutex is not locked or locked recursively");
+ }
+
+ /* The mutex must be updated to the released state */
+ mutex->count = 0;
+ mutex->owner = 0;
- if (pSleepConditionVariableSRW(&cond->cond, &mutex->srw, timeout, 0) == FALSE) {
- if (GetLastError() == ERROR_TIMEOUT) {
- ret = SDL_MUTEX_TIMEDOUT;
+ if (pSleepConditionVariableSRW(&cond->cond, &mutex->srw, timeout, 0) == FALSE) {
+ if (GetLastError() == ERROR_TIMEOUT) {
+ ret = SDL_MUTEX_TIMEDOUT;
+ } else {
+ ret = SDL_SetError("SleepConditionVariableSRW() failed");
+ }
} else {
- ret = SDL_SetError("SleepConditionVariableSRW() failed");
+ ret = 0;
}
+
+ /* The mutex is owned by us again, regardless of status of the wait */
+ SDL_assert(mutex->count == 0 && mutex->owner == 0);
+ mutex->count = 1;
+ mutex->owner = GetCurrentThreadId();
} else {
- ret = 0;
- }
+ SDL_mutex_cs *mutex = (SDL_mutex_cs *)_mutex;
- /* The mutex is owned by us again, regardless of status of the wait */
- SDL_assert(mutex->count == 0 && mutex->owner == 0);
- mutex->count = 1;
- mutex->owner = GetCurrentThreadId();
+ SDL_assert(SDL_mutex_impl_active.Type == SDL_MUTEX_CS);
+
+ if (pSleepConditionVariableCS(&cond->cond, &mutex->cs, timeout) == FALSE) {
+ if (GetLastError() == ERROR_TIMEOUT) {
+ ret = SDL_MUTEX_TIMEDOUT;
+ } else {
+ ret = SDL_SetError("SleepConditionVariableCS() failed");
+ }
+ } else {
+ ret = 0;
+ }
+ }
return ret;
}
static int
-SDL_CondWait_srw(SDL_cond * cond, SDL_mutex * mutex) {
- return SDL_CondWaitTimeout_srw(cond, mutex, SDL_MUTEX_MAXWAIT);
+SDL_CondWait_cv(SDL_cond * cond, SDL_mutex * mutex) {
+ return SDL_CondWaitTimeout_cv(cond, mutex, SDL_MUTEX_MAXWAIT);
}
-static const SDL_cond_impl_t SDL_cond_impl_srw =
+static const SDL_cond_impl_t SDL_cond_impl_cv =
{
- &SDL_CreateCond_srw,
- &SDL_DestroyCond_srw,
- &SDL_CondSignal_srw,
- &SDL_CondBroadcast_srw,
- &SDL_CondWait_srw,
- &SDL_CondWaitTimeout_srw,
+ &SDL_CreateCond_cv,
+ &SDL_DestroyCond_cv,
+ &SDL_CondSignal_cv,
+ &SDL_CondBroadcast_cv,
+ &SDL_CondWait_cv,
+ &SDL_CondWaitTimeout_cv,
};
/**
@@ -222,27 +242,24 @@ SDL_CreateCond(void)
SDL_assert(SDL_mutex_impl_active.Type != SDL_MUTEX_INVALID);
}
- /* It is required SRW Locks are used */
- if (SDL_mutex_impl_active.Type == SDL_MUTEX_SRW) {
#if __WINRT__
- /* Link statically on this platform */
- impl = &SDL_cond_impl_srw;
+ /* Link statically on this platform */
+ impl = &SDL_cond_impl_cv;
#else
+ {
HMODULE kernel32 = GetModuleHandle(TEXT("kernel32.dll"));
if (kernel32) {
pWakeConditionVariable = (pfnWakeConditionVariable) GetProcAddress(kernel32, "WakeConditionVariable");
pWakeAllConditionVariable = (pfnWakeAllConditionVariable) GetProcAddress(kernel32, "WakeAllConditionVariable");
pSleepConditionVariableSRW = (pfnSleepConditionVariableSRW) GetProcAddress(kernel32, "SleepConditionVariableSRW");
- if (pWakeConditionVariable && pWakeAllConditionVariable && pSleepConditionVariableSRW) {
+ pSleepConditionVariableCS = (pfnSleepConditionVariableCS) GetProcAddress(kernel32, "SleepConditionVariableCS");
+ if (pWakeConditionVariable && pWakeAllConditionVariable && pSleepConditionVariableSRW && pSleepConditionVariableCS) {
/* Use the Windows provided API */
- impl = &SDL_cond_impl_srw;
+ impl = &SDL_cond_impl_cv;
}
}
- if (!(kernel32 && pWakeConditionVariable && pWakeAllConditionVariable && pSleepConditionVariableSRW)) {
- SDL_LogWarn(SDL_LOG_CATEGORY_SYSTEM, "Could not load required imports for SRW Condition Variables although SRW Locks are used!");
- }
-#endif
}
+#endif
SDL_memcpy(&SDL_cond_impl_active, impl, sizeof(SDL_cond_impl_active));
}
diff --git a/src/thread/windows/SDL_sysmutex.c b/src/thread/windows/SDL_sysmutex.c
index 6bf9f83c22..e4918ded04 100644
--- a/src/thread/windows/SDL_sysmutex.c
+++ b/src/thread/windows/SDL_sysmutex.c
@@ -169,11 +169,6 @@ static const SDL_mutex_impl_t SDL_mutex_impl_srw =
* Fallback Mutex implementation using Critical Sections (before Win 7)
*/
-typedef struct SDL_mutex_cs
-{
- CRITICAL_SECTION cs;
-} SDL_mutex_cs;
-
/* Create a mutex */
static SDL_mutex *
SDL_CreateMutex_cs(void)
diff --git a/src/thread/windows/SDL_sysmutex_c.h b/src/thread/windows/SDL_sysmutex_c.h
index 6480e94211..1367ca635b 100644
--- a/src/thread/windows/SDL_sysmutex_c.h
+++ b/src/thread/windows/SDL_sysmutex_c.h
@@ -66,4 +66,9 @@ typedef struct SDL_mutex_srw
DWORD owner;
} SDL_mutex_srw;
+typedef struct SDL_mutex_cs
+{
+ CRITICAL_SECTION cs;
+} SDL_mutex_cs;
+
/* vi: set ts=4 sw=4 expandtab: */