From 899eb0d04216fcef77b0c9f2fad5acfd4d35234c Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Wed, 25 Oct 2023 10:00:26 -0400
Subject: [PATCH] thread: Locking mutexes and rwlocks are now void functions.
Almost nothing checks these return values, and there's no reason a valid
lock should fail to operate. The cases where a lock isn't valid (it's a
bogus pointer, it was previously destroyed, a thread is unlocking a lock it
doesn't own, etc) are undefined behavior and always were, and should be
treated as an application bug.
Reference Issue #8096.
---
docs/README-migration.md | 2 +
include/SDL3/SDL_mutex.h | 56 +++++----
src/SDL_properties.c | 66 +++++------
src/audio/SDL_audiocvt.c | 12 +-
src/dynapi/SDL_dynapi_procs.h | 10 +-
src/thread/generic/SDL_sysmutex.c | 135 +++++++++-------------
src/thread/generic/SDL_sysrwlock.c | 122 +++++++++-----------
src/thread/generic/SDL_sysrwlock_c.h | 10 +-
src/thread/n3ds/SDL_sysmutex.c | 40 ++-----
src/thread/ngage/SDL_sysmutex.cpp | 28 ++---
src/thread/psp/SDL_sysmutex.c | 101 ++++++-----------
src/thread/pthread/SDL_sysmutex.c | 150 +++++++++++--------------
src/thread/pthread/SDL_sysrwlock.c | 53 ++++-----
src/thread/stdcpp/SDL_sysmutex.cpp | 59 +++-------
src/thread/stdcpp/SDL_sysrwlock.cpp | 90 ++++++---------
src/thread/vita/SDL_sysmutex.c | 89 +++++----------
src/thread/windows/SDL_sysmutex.c | 81 +++++--------
src/thread/windows/SDL_sysmutex_c.h | 4 +-
src/thread/windows/SDL_sysrwlock_srw.c | 88 ++++++---------
test/testlock.c | 11 +-
test/testrwlock.c | 23 ++--
21 files changed, 490 insertions(+), 740 deletions(-)
diff --git a/docs/README-migration.md b/docs/README-migration.md
index ae1693de7c9b..aa1b044f1209 100644
--- a/docs/README-migration.md
+++ b/docs/README-migration.md
@@ -707,6 +707,8 @@ The following functions have been renamed:
## SDL_mutex.h
+SDL_LockMutex and SDL_UnlockMutex now return void; if the mutex is valid (including being a NULL pointer, which returns immediately), these functions never fail. If the mutex is invalid or the caller does something illegal, like unlock another thread's mutex, this is considered undefined behavior.
+
The following functions have been renamed:
* SDL_CondBroadcast() => SDL_BroadcastCondition()
* SDL_CondSignal() => SDL_SignalCondition()
diff --git a/include/SDL3/SDL_mutex.h b/include/SDL3/SDL_mutex.h
index 23fb86c30290..7dd150259f67 100644
--- a/include/SDL3/SDL_mutex.h
+++ b/include/SDL3/SDL_mutex.h
@@ -169,13 +169,15 @@ extern DECLSPEC SDL_Mutex *SDLCALL SDL_CreateMutex(void);
* unlock it the same number of times before it is actually made available for
* other threads in the system (this is known as a "recursive mutex").
*
+ * This function does not fail; if mutex is NULL, it will return immediately
+ * having locked nothing. If the mutex is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
* \param mutex the mutex to lock
- * \returns 0 on success or a negative error code on failure; call
- * SDL_GetError() for more information.
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
+extern DECLSPEC void SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
/**
* Try to lock a mutex without blocking.
@@ -186,9 +188,13 @@ extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
* This technique is useful if you need exclusive access to a resource but
* don't want to wait for it, and will return to it to try again later.
*
+ * This function does not fail; if mutex is NULL, it will return 0 immediately
+ * having locked nothing. If the mutex is valid, this function will always
+ * either lock the mutex and return 0, or return SDL_MUTEX_TIMEOUT and lock
+ * nothing.
+ *
* \param mutex the mutex to try to lock
- * \returns 0, `SDL_MUTEX_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- * more information.
+ * \returns 0 or `SDL_MUTEX_TIMEDOUT`
*
* \since This function is available since SDL 3.0.0.
*
@@ -210,12 +216,10 @@ extern DECLSPEC int SDLCALL SDL_TryLockMutex(SDL_Mutex *mutex) SDL_TRY_ACQUIRE(0
* thread, and doing so results in undefined behavior.
*
* \param mutex the mutex to unlock.
- * \returns 0 on success or a negative error code on failure; call
- * SDL_GetError() for more information.
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC int SDLCALL SDL_UnlockMutex(SDL_Mutex *mutex) SDL_RELEASE(mutex);
+extern DECLSPEC void SDLCALL SDL_UnlockMutex(SDL_Mutex *mutex) SDL_RELEASE(mutex);
/**
* Destroy a mutex created with SDL_CreateMutex().
@@ -321,15 +325,17 @@ extern DECLSPEC SDL_RWLock *SDLCALL SDL_CreateRWLock(void);
* lock before requesting a read-only lock. (But, of course, if you have the
* write lock, you don't need further locks to read in any case.)
*
+ * This function does not fail; if rwlock is NULL, it will return immediately
+ * having locked nothing. If the rwlock is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
* \param rwlock the read/write lock to lock
- * \returns 0 on success or a negative error code on failure; call
- * SDL_GetError() for more information.
*
* \since This function is available since SDL 3.0.0.
*
* \sa SDL_UnlockRWLock
*/
-extern DECLSPEC int SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQUIRE_SHARED(rwlock);
+extern DECLSPEC void SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQUIRE_SHARED(rwlock);
/**
* Lock the read/write lock for _write_ operations.
@@ -348,15 +354,17 @@ extern DECLSPEC int SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQ
* read-only lock. Doing so results in undefined behavior. Unlock the
* read-only lock before requesting a write lock.
*
+ * This function does not fail; if rwlock is NULL, it will return immediately
+ * having locked nothing. If the rwlock is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
* \param rwlock the read/write lock to lock
- * \returns 0 on success or a negative error code on failure; call
- * SDL_GetError() for more information.
*
* \since This function is available since SDL 3.0.0.
*
* \sa SDL_UnlockRWLock
*/
-extern DECLSPEC int SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQUIRE(rwlock);
+extern DECLSPEC void SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQUIRE(rwlock);
/**
* Try to lock a read/write lock _for reading_ without blocking.
@@ -370,9 +378,13 @@ extern DECLSPEC int SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQ
* Trying to lock for read-only access can succeed if other threads are
* holding read-only locks, as this won't prevent access.
*
+ * This function does not fail; if rwlock is NULL, it will return 0 immediately
+ * having locked nothing. If rwlock is valid, this function will always
+ * either lock the rwlock and return 0, or return SDL_RWLOCK_TIMEOUT and lock
+ * nothing.
+ *
* \param rwlock the rwlock to try to lock
- * \returns 0, `SDL_RWLOCK_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- * more information.
+ * \returns 0 or `SDL_RWLOCK_TIMEDOUT`
*
* \since This function is available since SDL 3.0.0.
*
@@ -400,9 +412,13 @@ extern DECLSPEC int SDLCALL SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) SDL_
* read-only lock. Doing so results in undefined behavior. Unlock the
* read-only lock before requesting a write lock.
*
+ * This function does not fail; if rwlock is NULL, it will return 0 immediately
+ * having locked nothing. If rwlock is valid, this function will always
+ * either lock the rwlock and return 0, or return SDL_RWLOCK_TIMEOUT and lock
+ * nothing.
+ *
* \param rwlock the rwlock to try to lock
- * \returns 0, `SDL_RWLOCK_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- * more information.
+ * \returns 0 or `SDL_RWLOCK_TIMEDOUT`
*
* \since This function is available since SDL 3.0.0.
*
@@ -428,12 +444,10 @@ extern DECLSPEC int SDLCALL SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) SDL_
* thread, and doing so results in undefined behavior.
*
* \param rwlock the rwlock to unlock.
- * \returns 0 on success or a negative error code on failure; call
- * SDL_GetError() for more information.
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC int SDLCALL SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_RELEASE_GENERIC(rwlock);
+extern DECLSPEC void SDLCALL SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_RELEASE_GENERIC(rwlock);
/**
* Destroy a read/write lock created with SDL_CreateRWLock().
diff --git a/src/SDL_properties.c b/src/SDL_properties.c
index 22613752d3d9..0b8d79534781 100644
--- a/src/SDL_properties.c
+++ b/src/SDL_properties.c
@@ -123,17 +123,16 @@ SDL_PropertiesID SDL_CreateProperties(void)
goto error;
}
- if (SDL_LockRWLockForWriting(SDL_properties_lock) == 0) {
+ SDL_LockRWLockForWriting(SDL_properties_lock);
+ ++SDL_last_properties_id;
+ if (SDL_last_properties_id == 0) {
++SDL_last_properties_id;
- if (SDL_last_properties_id == 0) {
- ++SDL_last_properties_id;
- }
- props = SDL_last_properties_id;
- if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) {
- inserted = SDL_TRUE;
- }
- SDL_UnlockRWLock(SDL_properties_lock);
}
+ props = SDL_last_properties_id;
+ if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) {
+ inserted = SDL_TRUE;
+ }
+ SDL_UnlockRWLock(SDL_properties_lock);
if (inserted) {
/* All done! */
@@ -152,15 +151,17 @@ int SDL_LockProperties(SDL_PropertiesID props)
if (!props) {
return SDL_InvalidParamError("props");
}
- if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
- SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
- SDL_UnlockRWLock(SDL_properties_lock);
- }
+
+ SDL_LockRWLockForReading(SDL_properties_lock);
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ SDL_UnlockRWLock(SDL_properties_lock);
+
if (!properties) {
- SDL_InvalidParamError("props");
- return -1;
+ return SDL_InvalidParamError("props");
}
- return SDL_LockMutex(properties->lock);
+
+ SDL_LockMutex(properties->lock);
+ return 0;
}
void SDL_UnlockProperties(SDL_PropertiesID props)
@@ -170,13 +171,15 @@ void SDL_UnlockProperties(SDL_PropertiesID props)
if (!props) {
return;
}
- if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
- SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
- SDL_UnlockRWLock(SDL_properties_lock);
- }
+
+ SDL_LockRWLockForReading(SDL_properties_lock);
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ SDL_UnlockRWLock(SDL_properties_lock);
+
if (!properties) {
return;
}
+
SDL_UnlockMutex(properties->lock);
}
@@ -193,10 +196,10 @@ int SDL_SetProperty(SDL_PropertiesID props, const char *name, void *value, void
return SDL_InvalidParamError("name");
}
- if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
- SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
- SDL_UnlockRWLock(SDL_properties_lock);
- }
+ SDL_LockRWLockForReading(SDL_properties_lock);
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ SDL_UnlockRWLock(SDL_properties_lock);
+
if (!properties) {
return SDL_InvalidParamError("props");
}
@@ -242,10 +245,10 @@ void *SDL_GetProperty(SDL_PropertiesID props, const char *name)
return NULL;
}
- if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
- SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
- SDL_UnlockRWLock(SDL_properties_lock);
- }
+ SDL_LockRWLockForReading(SDL_properties_lock);
+ SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+ SDL_UnlockRWLock(SDL_properties_lock);
+
if (!properties) {
SDL_InvalidParamError("props");
return NULL;
@@ -280,8 +283,7 @@ void SDL_DestroyProperties(SDL_PropertiesID props)
return;
}
- if (SDL_LockRWLockForWriting(SDL_properties_lock) == 0) {
- SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props);
- SDL_UnlockRWLock(SDL_properties_lock);
- }
+ SDL_LockRWLockForWriting(SDL_properties_lock);
+ SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props);
+ SDL_UnlockRWLock(SDL_properties_lock);
}
diff --git a/src/audio/SDL_audiocvt.c b/src/audio/SDL_audiocvt.c
index 7681e76cb9cd..b64ca480dc84 100644
--- a/src/audio/SDL_audiocvt.c
+++ b/src/audio/SDL_audiocvt.c
@@ -476,12 +476,20 @@ int SDL_SetAudioStreamPutCallback(SDL_AudioStream *stream, SDL_AudioStreamCallba
int SDL_LockAudioStream(SDL_AudioStream *stream)
{
- return stream ? SDL_LockMutex(stream->lock) : SDL_InvalidParamError("stream");
+ if (!stream) {
+ return SDL_InvalidParamError("stream");
+ }
+ SDL_LockMutex(stream->lock);
+ return 0;
}
int SDL_UnlockAudioStream(SDL_AudioStream *stream)
{
- return stream ? SDL_UnlockMutex(stream->lock) : SDL_InvalidParamError("stream");
+ if (!stream) {
+ return SDL_InvalidParamError("stream");
+ }
+ SDL_UnlockMutex(stream->lock);
+ return 0;
}
int SDL_GetAudioStreamFormat(SDL_AudioStream *stream, SDL_AudioSpec *src_spec, SDL_AudioSpec *dst_spec)
diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h
index b8ac62c583b0..ac41615e9de1 100644
--- a/src/dynapi/SDL_dynapi_procs.h
+++ b/src/dynapi/SDL_dynapi_procs.h
@@ -519,9 +519,9 @@ SDL_DYNAPI_PROC(void*,SDL_LoadFile_RW,(SDL_RWops *a, size_t *b, SDL_bool c),(a,b
SDL_DYNAPI_PROC(SDL_FunctionPointer,SDL_LoadFunction,(void *a, const char *b),(a,b),return)
SDL_DYNAPI_PROC(void*,SDL_LoadObject,(const char *a),(a),return)
SDL_DYNAPI_PROC(void,SDL_LockJoysticks,(void),(),)
-SDL_DYNAPI_PROC(int,SDL_LockMutex,(SDL_Mutex *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_LockRWLockForReading,(SDL_RWLock *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_LockRWLockForWriting,(SDL_RWLock *a),(a),return)
+SDL_DYNAPI_PROC(void,SDL_LockMutex,(SDL_Mutex *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_LockRWLockForReading,(SDL_RWLock *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_LockRWLockForWriting,(SDL_RWLock *a),(a),)
SDL_DYNAPI_PROC(int,SDL_LockSurface,(SDL_Surface *a),(a),return)
SDL_DYNAPI_PROC(int,SDL_LockTexture,(SDL_Texture *a, const SDL_Rect *b, void **c, int *d),(a,b,c,d),return)
SDL_DYNAPI_PROC(int,SDL_LockTextureToSurface,(SDL_Texture *a, const SDL_Rect *b, SDL_Surface **c),(a,b,c),return)
@@ -706,8 +706,8 @@ SDL_DYNAPI_PROC(int,SDL_TryLockRWLockForWriting,(SDL_RWLock *a),(a),return)
SDL_DYNAPI_PROC(int,SDL_TryWaitSemaphore,(SDL_Semaphore *a),(a),return)
SDL_DYNAPI_PROC(void,SDL_UnloadObject,(void *a),(a),)
SDL_DYNAPI_PROC(void,SDL_UnlockJoysticks,(void),(),)
-SDL_DYNAPI_PROC(int,SDL_UnlockMutex,(SDL_Mutex *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_UnlockRWLock,(SDL_RWLock *a),(a),return)
+SDL_DYNAPI_PROC(void,SDL_UnlockMutex,(SDL_Mutex *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_UnlockRWLock,(SDL_RWLock *a),(a),)
SDL_DYNAPI_PROC(void,SDL_UnlockSurface,(SDL_Surface *a),(a),)
SDL_DYNAPI_PROC(void,SDL_UnlockTexture,(SDL_Texture *a),(a),)
SDL_DYNAPI_PROC(void,SDL_UpdateGamepads,(void),(),)
diff --git a/src/thread/generic/SDL_sysmutex.c b/src/thread/generic/SDL_sysmutex.c
index 8d4a3702d1eb..579d028ee3ef 100644
--- a/src/thread/generic/SDL_sysmutex.c
+++ b/src/thread/generic/SDL_sysmutex.c
@@ -20,7 +20,7 @@
*/
#include "SDL_internal.h"
-/* An implementation of mutexes using semaphores */
+// An implementation of mutexes using semaphores
#include "SDL_systhread_c.h"
@@ -31,13 +31,9 @@ struct SDL_Mutex
SDL_Semaphore *sem;
};
-/* Create a mutex */
SDL_Mutex *SDL_CreateMutex(void)
{
- SDL_Mutex *mutex;
-
- /* Allocate mutex memory */
- mutex = (SDL_Mutex *)SDL_calloc(1, sizeof(*mutex));
+ SDL_Mutex *mutex = (SDL_Mutex *)SDL_calloc(1, sizeof(*mutex));
#ifndef SDL_THREADS_DISABLED
if (mutex) {
@@ -52,12 +48,11 @@ SDL_Mutex *SDL_CreateMutex(void)
} else {
SDL_OutOfMemory();
}
-#endif /* !SDL_THREADS_DISABLED */
+#endif // !SDL_THREADS_DISABLED
return mutex;
}
-/* Free the mutex */
void SDL_DestroyMutex(SDL_Mutex *mutex)
{
if (mutex) {
@@ -68,94 +63,72 @@ void SDL_DestroyMutex(SDL_Mutex *mutex)
}
}
-/* Lock the mutex */
-int SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes
{
-#ifdef SDL_THREADS_DISABLED
- return 0;
-#else
- SDL_threadID this_thread;
-
- if (mutex == NULL) {
- return 0;
- }
-
- this_thread = SDL_ThreadID();
- if (mutex->owner == this_thread) {
- ++mutex->recursive;
- } else {
- /* The order of operations is important.
- We set the locking thread id after we obtain the lock
- so unlocks from other threads will fail.
- */
- SDL_WaitSemaphore(mutex->sem);
- mutex->owner = this_thread;
- mutex->recursive = 0;
+#ifndef SDL_THREADS_DISABLED
+ if (mutex != NULL) {
+ SDL_threadID this_thread = SDL_ThreadID();
+ if (mutex->owner == this_thread) {
+ ++mutex->recursive;
+ } else {
+ /* The order of operations is important.
+ We set the locking thread id after we obtain the lock
+ so unlocks from other threads will fail.
+ */
+ SDL_WaitSemaphore(mutex->sem);
+ mutex->owner = this_thread;
+ mutex->recursive = 0;
+ }
}
-
- return 0;
#endif /* SDL_THREADS_DISABLED */
}
-/* try Lock the mutex */
int SDL_TryLockMutex(SDL_Mutex *mutex)
{
-#ifdef SDL_THREADS_DISABLED
- return 0;
-#else
int retval = 0;
- SDL_threadID this_thread;
-
- if (mutex == NULL) {
- return 0;
- }
-
- this_thread = SDL_ThreadID();
- if (mutex->owner == this_thread) {
- ++mutex->recursive;
- } else {
- /* The order of operations is important.
- We set the locking thread id after we obtain the lock
- so unlocks from other threads will fail.
- */
- retval = SDL_TryWaitSemaphore(mutex->sem);
- if (retval == 0) {
- mutex->owner = this_thread;
- mutex->recursive = 0;
+#ifndef SDL_THREADS_DISABLED
+ if (mutex != NULL) {
+ SDL_threadID this_thread = SDL_ThreadID();
+ if (mutex->owner == this_thread) {
+ ++mutex->recursive;
+ } else {
+ /* The order of operations is important.
+ We set the locking thread id after we obtain the lock
+ so unlocks from other threads will fail.
+ */
+ retval = SDL_TryWaitSemaphore(mutex->sem);
+ if (retval == 0) {
+ mutex->owner = this_thread;
+ mutex->recursive = 0;
+ }
}
}
-
+#endif // SDL_THREADS_DISABLED
return retval;
-#endif /* SDL_THREADS_DISABLED */
}
-/* Unlock the mutex */
-int SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes
{
-#ifdef SDL_THREADS_DISABLED
- return 0;
-#else
- if (mutex == NULL) {
- return 0;
- }
-
- /* If we don't own the mutex, we can't unlock it */
- if (SDL_ThreadID() != mutex->owner) {
- return SDL_SetError("mutex not owned by this thread");
- }
+#ifndef SDL_THREADS_DISABLED
+ if (mutex != NULL) {
+ // If we don't own the mutex, we can't unlock it
+ if (SDL_ThreadID() != mutex->owner) {
+ SDL_assert(!"Tried to unlock a mutex we don't own!");
+ return; // (undefined behavior!) SDL_SetError("mutex not owned by this thread");
+ }
- if (mutex->recursive) {
- --mutex->recursive;
- } else {
- /* The order of operations is important.
- First reset the owner so another thread doesn't lock
- the mutex and set the ownership before we reset it,
- then release the lock semaphore.
- */
- mutex->owner = 0;
- SDL_PostSemaphore(mutex->sem);
+ if (mutex->recursive) {
+ --mutex->recursive;
+ } else {
+ /* The order of operations is important.
+ First reset the owner so another thread doesn't lock
+ the mutex and set the ownership before we reset it,
+ then release the lock semaphore.
+ */
+ mutex->owner = 0;
+ SDL_PostSemaphore(mutex->sem);
+ }
}
- return 0;
-#endif /* SDL_THREADS_DISABLED */
+#endif // SDL_THREADS_DISABLED
}
diff --git a/src/thread/generic/SDL_sysrwlock.c b/src/thread/generic/SDL_sysrwlock.c
index d88bee15d380..a38b81d6ac76 100644
--- a/src/thread/generic/SDL_sysrwlock.c
+++ b/src/thread/generic/SDL_sysrwlock.c
@@ -20,7 +20,7 @@
*/
#include "SDL_internal.h"
-/* An implementation of rwlocks using mutexes, condition variables, and atomics. */
+// An implementation of rwlocks using mutexes, condition variables, and atomics.
#include "SDL_systhread_c.h"
@@ -30,7 +30,7 @@
* will be chosen at runtime), the function names need to be
* suffixed
*/
-/* !!! FIXME: this is quite a tapdance with macros and the build system, maybe we can simplify how we do this. --ryan. */
+// !!! FIXME: this is quite a tapdance with macros and the build system, maybe we can simplify how we do this. --ryan.
#ifndef SDL_THREAD_GENERIC_RWLOCK_SUFFIX
#define SDL_CreateRWLock_generic SDL_CreateRWLock
#define SDL_DestroyRWLock_generic SDL_DestroyRWLock
@@ -95,63 +95,48 @@ void SDL_DestroyRWLock_generic(SDL_RWLock *rwlock)
}
}
-int SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes
{
#ifndef SDL_THREADS_DISABLED
- if (!rwlock) {
- return SDL_InvalidParamError("rwlock");
- } else if (SDL_LockMutex(rwlock->lock) == -1) {
- return -1;
+ if (rwlock) {
+ // !!! FIXME: these don't have to be atomic, we always gate them behind a mutex.
+ SDL_LockMutex(rwlock->lock);
+ SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0); // shouldn't be able to grab lock if there's a writer!
+ SDL_AtomicAdd(&rwlock->reader_count, 1);
+ SDL_UnlockMutex(rwlock->lock); // other readers can attempt to share the lock.
}
-
- SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0); /* shouldn't be able to grab lock if there's a writer! */
-
- SDL_AtomicAdd(&rwlock->reader_count, 1);
- SDL_UnlockMutex(rwlock->lock); /* other readers can attempt to share the lock. */
#endif
-
- return 0;
}
-int SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes
{
#ifndef SDL_THREADS_DISABLED
- if (!rwlock) {
- return SDL_InvalidParamError("rwlock");
- } else if (SDL_LockMutex(rwlock->lock) == -1) {
- return -1;
- }
+ if (rwlock) {
+ SDL_LockMutex(rwlock->lock);
+ while (SDL_AtomicGet(&rwlock->reader_count) > 0) { // while something is holding the shared lock, keep waiting.
+ SDL_WaitCondition(rwlock->condition, rwlock->lock); // release the lock and wait for readers holding the shared lock to release it, regrab the lock.
+ }
- while (SDL_AtomicGet(&rwlock->reader_count) > 0) { /* while something is holding the shared lock, keep waiting. */
- SDL_WaitCondition(rwlock->condition, rwlock->lock); /* release the lock and wait for readers holding the shared lock to release it, regrab the lock. */
+ // we hold the lock!
+ SDL_AtomicAdd(&rwlock->writer_count, 1); // we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly!
}
-
- /* we hold the lock! */
- SDL_AtomicAdd(&rwlock->writer_count, 1); /* we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly! */
#endif
-
- return 0;
}
int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock)
{
#ifndef SDL_THREADS_DISABLED
- int rc;
-
- if (!rwlock) {
- return SDL_InvalidParamError("rwlock");
- }
-
- rc = SDL_TryLockMutex(rwlock->lock);
- if (rc != 0) {
- /* !!! FIXME: there is a small window where a reader has to lock the mutex, and if we hit that, we will return SDL_RWLOCK_TIMEDOUT even though we could have shared the lock. */
- return rc;
+ if (rwlock) {
+ const int rc = SDL_TryLockMutex(rwlock->lock);
+ if (rc != 0) {
+ // !!! FIXME: there is a small window where a reader has to lock the mutex, and if we hit that, we will return SDL_RWLOCK_TIMEDOUT even though we could have shared the lock.
+ return rc;
+ }
+
+ SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0); // shouldn't be able to grab lock if there's a writer!
+ SDL_AtomicAdd(&rwlock->reader_count, 1);
+ SDL_UnlockMutex(rwlock->lock); // other readers can attempt to share the lock.
}
-
- SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0); /* shouldn't be able to grab lock if there's a writer! */
-
- SDL_AtomicAdd(&rwlock->reader_count, 1);
- SDL_UnlockMutex(rwlock->lock); /* other readers can attempt to share the lock. */
#endif
return 0;
@@ -160,46 +145,41 @@ int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock)
int SDL_TryLockRWLockForWriting_generic(SDL_RWLock *rwlock)
{
#ifndef SDL_THREADS_DISABLED
- int rc;
-
- if (!rwlock) {
- return SDL_InvalidParamError("rwlock");
- } else if ((rc = SDL_TryLockMutex(rwlock->lock)) != 0) {
- return rc;
- }
-
- if (SDL_AtomicGet(&rwlock->reader_count) > 0) { /* a reader is using the shared lock, treat it as unavailable. */
- SDL_UnlockMutex(rwlock->lock);
- return SDL_RWLOCK_TIMEDOUT;
+ if (rwlock) {
+ const int rc = SDL_TryLockMutex(rwlock->lock);
+ if (rc != 0) {
+ return rc;
+ }
+
+ if (SDL_AtomicGet(&rwlock->reader_count) > 0) { // a reader is using the shared lock, treat it as unavailable.
+ SDL_UnlockMutex(rwlock->lock);
+ return SDL_RWLOCK_TIMEDOUT;
+ }
+
+ // we hold the lock!
+ SDL_AtomicAdd(&rwlock->writer_count, 1); // we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly!
}
-
- /* we hold the lock! */
- SDL_AtomicAdd(&rwlock->writer_count, 1); /* we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly! */
#endif
return 0;
}
-int SDL_UnlockRWLock_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_UnlockRWLock_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes
{
#ifndef SDL_THREADS_DISABLED
- if (!rwlock) {
- return SDL_InvalidParamError("rwlock");
- }
+ if (rwlock) {
+ SDL_LockMutex(rwlock->lock); // recursive lock for writers, readers grab lock to make sure things are sane.
- SDL_LockMutex(rwlock->lock); /* recursive lock for writers, readers grab lock to make sure things are sane. */
+ if (SDL_AtomicGet(&rwlock->reader_count) > 0) { // we're a reader
+ SDL_AtomicAdd(&rwlock->reader_count, -1);
+ SDL_BroadcastCondition(rwlock->condition); // alert any pending writers to attempt to try to grab the lock again.
+ } else if (SDL_AtomicGet(&rwlock->writer_count) > 0) { // we're a writer
+ SDL_AtomicAdd(&rwlock->writer_count, -1);
+ SDL_UnlockMutex(rwlock->lock); // recursive unlock.
+ }
- if (SDL_AtomicGet(&rwlock->reader_count) > 0) { /* we're a reader */
- SDL_AtomicAdd(&rwlock->reader_count, -1);
- SDL_BroadcastCondition(rwlock->condition); /* alert any pending writers to attempt to try to grab the lock again. */
- } else if (SDL_AtomicGet(&rwlock->writer_count) > 0) { /* we're a writer */
- SDL_AtomicAdd(&rwlock->writer_count, -1);
- SDL_UnlockMutex(rwlock->lock); /* recursive unlock. */
+ SDL_UnlockMutex(rwlock->lock);
}
-
- SDL_UnlockMutex(rwlock->lock);
#endif
-
- return 0;
}
diff --git a/src/thread/generic/SDL_sysrwlock_c.h b/src/thread/generic/SDL_sysrwlock_c.h
index 8247f8ebe684..9083cc89c84e 100644
--- a/src/thread/generic/SDL_sysrwlock_c.h
+++ b/src/thread/generic/SDL_sysrwlock_c.h
@@ -27,12 +27,12 @@
SDL_RWLock *SDL_CreateRWLock_generic(void);
void SDL_DestroyRWLock_generic(SDL_RWLock *rwlock);
-int SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock);
-int SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock);
+void SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock);
+void SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock);
int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock);
int SDL_TryLockRWLockForWriting_generic(SDL_RWLock *rwlock);
-int SDL_UnlockRWLock_generic(SDL_RWLock *rwlock);
+void SDL_UnlockRWLock_generic(SDL_RWLock *rwlock);
-#endif /* SDL_THREAD_GENERIC_RWLOCK_SUFFIX */
+#endif // SDL_THREAD_GENERIC_RWLOCK_SUFFIX
-#endif /* SDL_sysrwlock_c_h_ */
+#endif // SDL_sysrwlock_c_h_
diff --git a/src/thread/n3ds/SDL_sysmutex.c b/src/thread/n3ds/SDL_sysmutex.c
index 0995ad11f4a3..47e848582da2 100644
--- a/src/thread/n3ds/SDL_sysmutex.c
+++ b/src/thread/n3ds/SDL_sysmutex.c
@@ -22,17 +22,13 @@
#ifdef SDL_THREAD_N3DS
-/* An implementation of mutexes using libctru's RecursiveLock */
+// An implementation of mutexes using libctru's RecursiveLock
#include "SDL_sysmutex_c.h"
-/* Create a mutex */
SDL_Mutex *SDL_CreateMutex(void)
{
- SDL_Mutex *mutex;
-
- /* Allocate mutex memory */
- mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex));
+ SDL_Mutex *mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex));
if (mutex) {
RecursiveLock_Init(&mutex->lock);
} else {
@@ -41,7 +37,6 @@ SDL_Mutex *SDL_CreateMutex(void)
return mutex;
}
-/* Free the mutex */
void SDL_DestroyMutex(SDL_Mutex *mutex)
{
if (mutex) {
@@ -49,38 +44,23 @@ void SDL_DestroyMutex(SDL_Mutex *mutex)
}
}
-/* Lock the mutex */
-int SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSI
(Patch may be truncated, please check the link at the top of this post.)