From 5c29b58e954dddd7beba22a7df3efbc8d410abc4 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Tue, 13 Dec 2022 14:03:40 -0800
Subject: [PATCH] Added support for clang thread-safety analysis
The annotations have been added to SDL_mutex.h and have been made public so applications can enable this for their own code.
Clang assumes that locking and unlocking can't fail, but SDL has the concept of a NULL mutex, so the mutex functions have been changed not to report errors if a mutex hasn't been initialized. We do have mutexes that might be accessed when they are NULL, notably in the event system, so this is an important change.
This commit cleans up a bunch of rare race conditions in the joystick and game controller code so now everything should be completely protected by the joystick lock.
To test this, change the compiler to "clang -Wthread-safety -Werror=thread-safety -DSDL_THREAD_SAFETY_ANALYSIS"
---
include/SDL3/SDL_joystick.h | 8 +-
include/SDL3/SDL_mutex.h | 80 +-
src/SDL_assert.c | 6 +-
src/SDL_log.c | 10 +-
src/audio/SDL_audio.c | 4 +-
src/dynapi/SDL_dynapi.h | 2 +-
src/events/SDL_events.c | 188 ++--
src/haptic/SDL_haptic.c | 88 +-
src/haptic/linux/SDL_syshaptic.c | 6 +
src/joystick/SDL_gamecontroller.c | 1031 ++++++++++++--------
src/joystick/SDL_joystick.c | 769 ++++++++-------
src/joystick/SDL_joystick_c.h | 2 +-
src/joystick/SDL_sysjoystick.h | 78 +-
src/joystick/hidapi/SDL_hidapi_combined.c | 2 +
src/joystick/hidapi/SDL_hidapi_gamecube.c | 7 +
src/joystick/hidapi/SDL_hidapi_luna.c | 2 +
src/joystick/hidapi/SDL_hidapi_ps3.c | 4 +
src/joystick/hidapi/SDL_hidapi_ps4.c | 2 +
src/joystick/hidapi/SDL_hidapi_ps5.c | 4 +-
src/joystick/hidapi/SDL_hidapi_rumble.c | 31 +-
src/joystick/hidapi/SDL_hidapi_rumble.h | 11 +-
src/joystick/hidapi/SDL_hidapi_shield.c | 4 +-
src/joystick/hidapi/SDL_hidapi_stadia.c | 2 +
src/joystick/hidapi/SDL_hidapi_steam.c | 2 +
src/joystick/hidapi/SDL_hidapi_switch.c | 6 +-
src/joystick/hidapi/SDL_hidapi_wii.c | 4 +-
src/joystick/hidapi/SDL_hidapi_xbox360.c | 2 +
src/joystick/hidapi/SDL_hidapi_xbox360w.c | 2 +
src/joystick/hidapi/SDL_hidapi_xboxone.c | 10 +-
src/joystick/hidapi/SDL_hidapijoystick.c | 44 +-
src/joystick/linux/SDL_sysjoystick.c | 31 +-
src/joystick/virtual/SDL_virtualjoystick.c | 53 +-
src/sensor/SDL_sensor.c | 51 +-
src/thread/generic/SDL_sysmutex.c | 10 +-
src/thread/n3ds/SDL_sysmutex.c | 10 +-
src/thread/ngage/SDL_sysmutex.cpp | 36 +-
src/thread/psp/SDL_sysmutex.c | 47 +-
src/thread/pthread/SDL_sysmutex.c | 10 +-
src/thread/stdcpp/SDL_sysmutex.cpp | 13 +-
src/thread/vita/SDL_sysmutex.c | 46 +-
src/thread/windows/SDL_sysmutex.c | 20 +-
41 files changed, 1611 insertions(+), 1127 deletions(-)
diff --git a/include/SDL3/SDL_joystick.h b/include/SDL3/SDL_joystick.h
index 5fa2e754ee02..23cba8e2274e 100644
--- a/include/SDL3/SDL_joystick.h
+++ b/include/SDL3/SDL_joystick.h
@@ -44,6 +44,7 @@
#include <SDL3/SDL_stdinc.h>
#include <SDL3/SDL_error.h>
#include <SDL3/SDL_guid.h>
+#include <SDL3/SDL_mutex.h>
#include <SDL3/begin_code.h>
/* Set up for C function definitions, even when using C++ */
@@ -66,6 +67,9 @@ extern "C" {
/**
* The joystick structure used to identify an SDL joystick
*/
+#ifdef SDL_THREAD_SAFETY_ANALYSIS
+extern SDL_mutex *SDL_joystick_lock;
+#endif
struct _SDL_Joystick;
typedef struct _SDL_Joystick SDL_Joystick;
@@ -131,7 +135,7 @@ typedef enum
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC void SDLCALL SDL_LockJoysticks(void);
+extern DECLSPEC void SDLCALL SDL_LockJoysticks(void) SDL_ACQUIRE(SDL_joystick_lock);
/**
@@ -146,7 +150,7 @@ extern DECLSPEC void SDLCALL SDL_LockJoysticks(void);
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC void SDLCALL SDL_UnlockJoysticks(void);
+extern DECLSPEC void SDLCALL SDL_UnlockJoysticks(void) SDL_RELEASE(SDL_joystick_lock);
/**
* Count the number of joysticks attached to the system.
diff --git a/include/SDL3/SDL_mutex.h b/include/SDL3/SDL_mutex.h
index de35462485bc..a3b2bc35b348 100644
--- a/include/SDL3/SDL_mutex.h
+++ b/include/SDL3/SDL_mutex.h
@@ -31,6 +31,80 @@
#include <SDL3/SDL_stdinc.h>
#include <SDL3/SDL_error.h>
+/******************************************************************************/
+/* Enable thread safety attributes only with clang.
+ * The attributes can be safely erased when compiling with other compilers.
+ */
+#if defined(SDL_THREAD_SAFETY_ANALYSIS) && \
+ defined(__clang__) && (!defined(SWIG))
+#define SDL_THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
+#else
+#define SDL_THREAD_ANNOTATION_ATTRIBUTE__(x) // no-op
+#endif
+
+#define SDL_CAPABILITY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
+
+#define SDL_SCOPED_CAPABILITY \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+
+#define SDL_GUARDED_BY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+
+#define SDL_PT_GUARDED_BY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x))
+
+#define SDL_ACQUIRED_BEFORE(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__))
+
+#define SDL_ACQUIRED_AFTER(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__))
+
+#define SDL_REQUIRES(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__))
+
+#define SDL_REQUIRES_SHARED(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__))
+
+#define SDL_ACQUIRE(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__))
+
+#define SDL_ACQUIRE_SHARED(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__))
+
+#define SDL_RELEASE(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__))
+
+#define SDL_RELEASE_SHARED(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__))
+
+#define SDL_RELEASE_GENERIC(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(release_generic_capability(__VA_ARGS__))
+
+#define SDL_TRY_ACQUIRE(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__))
+
+#define SDL_TRY_ACQUIRE_SHARED(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__))
+
+#define SDL_EXCLUDES(...) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__))
+
+#define SDL_ASSERT_CAPABILITY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x))
+
+#define SDL_ASSERT_SHARED_CAPABILITY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x))
+
+#define SDL_RETURN_CAPABILITY(x) \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+
+#define SDL_NO_THREAD_SAFETY_ANALYSIS \
+ SDL_THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+/******************************************************************************/
+
+
#include <SDL3/begin_code.h>
/* Set up for C function definitions, even when using C++ */
#ifdef __cplusplus
@@ -96,7 +170,7 @@ extern DECLSPEC SDL_mutex *SDLCALL SDL_CreateMutex(void);
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_mutex * mutex);
+extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_mutex * mutex) SDL_ACQUIRE(mutex);
#define SDL_mutexP(m) SDL_LockMutex(m)
/**
@@ -119,7 +193,7 @@ extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_mutex * mutex);
* \sa SDL_LockMutex
* \sa SDL_UnlockMutex
*/
-extern DECLSPEC int SDLCALL SDL_TryLockMutex(SDL_mutex * mutex);
+extern DECLSPEC int SDLCALL SDL_TryLockMutex(SDL_mutex * mutex) SDL_TRY_ACQUIRE(0, mutex);
/**
* Unlock the mutex.
@@ -138,7 +212,7 @@ extern DECLSPEC int SDLCALL SDL_TryLockMutex(SDL_mutex * mutex);
*
* \since This function is available since SDL 3.0.0.
*/
-extern DECLSPEC int SDLCALL SDL_UnlockMutex(SDL_mutex * mutex);
+extern DECLSPEC int SDLCALL SDL_UnlockMutex(SDL_mutex * mutex) SDL_RELEASE(mutex);
#define SDL_mutexV(m) SDL_UnlockMutex(m)
/**
diff --git a/src/SDL_assert.c b/src/SDL_assert.c
index ab360140c456..9eb532d63413 100644
--- a/src/SDL_assert.c
+++ b/src/SDL_assert.c
@@ -337,10 +337,8 @@ SDL_ReportAssertion(SDL_AssertData *data, const char *func, const char *file,
}
SDL_AtomicUnlock(&spinlock);
- if (SDL_LockMutex(assertion_mutex) < 0) {
- return SDL_ASSERTION_IGNORE; /* oh well, I guess. */
- }
-#endif
+ SDL_LockMutex(assertion_mutex);
+#endif /* !SDL_THREADS_DISABLED */
/* doing this because Visual C is upset over assigning in the macro. */
if (data->trigger_count == 0) {
diff --git a/src/SDL_log.c b/src/SDL_log.c
index 99d748954871..bdba9a94069b 100644
--- a/src/SDL_log.c
+++ b/src/SDL_log.c
@@ -333,15 +333,9 @@ void SDL_LogMessageV(int category, SDL_LogPriority priority, const char *fmt, va
}
}
- if (log_function_mutex) {
- SDL_LockMutex(log_function_mutex);
- }
-
+ SDL_LockMutex(log_function_mutex);
SDL_log_function(SDL_log_userdata, category, priority, message);
-
- if (log_function_mutex) {
- SDL_UnlockMutex(log_function_mutex);
- }
+ SDL_UnlockMutex(log_function_mutex);
/* Free only if dynamically allocated */
if (message != stack_buf) {
diff --git a/src/audio/SDL_audio.c b/src/audio/SDL_audio.c
index 2cc8fc5cef3e..83a92b5b2843 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -269,14 +269,14 @@ static SDL_INLINE SDL_bool is_in_audio_device_thread(SDL_AudioDevice *device)
return SDL_FALSE;
}
-static void SDL_AudioLockDevice_Default(SDL_AudioDevice *device)
+static void SDL_AudioLockDevice_Default(SDL_AudioDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang assumes recursive locks */
{
if (!is_in_audio_device_thread(device)) {
SDL_LockMutex(device->mixer_lock);
}
}
-static void SDL_AudioUnlockDevice_Default(SDL_AudioDevice *device)
+static void SDL_AudioUnlockDevice_Default(SDL_AudioDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang assumes recursive locks */
{
if (!is_in_audio_device_thread(device)) {
SDL_UnlockMutex(device->mixer_lock);
diff --git a/src/dynapi/SDL_dynapi.h b/src/dynapi/SDL_dynapi.h
index 18b27e76bbd4..5c1bce5065b4 100644
--- a/src/dynapi/SDL_dynapi.h
+++ b/src/dynapi/SDL_dynapi.h
@@ -57,7 +57,7 @@
#define SDL_DYNAMIC_API 0
#elif defined(__riscos__) && __riscos__ /* probably not useful on RISC OS, since dlopen() can't be used when using static linking. */
#define SDL_DYNAMIC_API 0
-#elif defined(__clang_analyzer__)
+#elif defined(__clang_analyzer__) || defined(SDL_THREAD_SAFETY_ANALYSIS)
#define SDL_DYNAMIC_API 0 /* Turn off for static analysis, so reports are more clear. */
#elif defined(__VITA__)
#define SDL_DYNAMIC_API 0 /* vitasdk doesn't support dynamic linking */
diff --git a/src/events/SDL_events.c b/src/events/SDL_events.c
index 6c88cf3d1ab7..46efed66e192 100644
--- a/src/events/SDL_events.c
+++ b/src/events/SDL_events.c
@@ -517,9 +517,7 @@ void SDL_StopEventLoop(void)
SDL_EventEntry *entry;
SDL_SysWMEntry *wmmsg;
- if (SDL_EventQ.lock) {
- SDL_LockMutex(SDL_EventQ.lock);
- }
+ SDL_LockMutex(SDL_EventQ.lock);
SDL_EventQ.active = SDL_FALSE;
@@ -576,8 +574,9 @@ void SDL_StopEventLoop(void)
}
SDL_zero(SDL_EventOK);
+ SDL_UnlockMutex(SDL_EventQ.lock);
+
if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
SDL_DestroyMutex(SDL_EventQ.lock);
SDL_EventQ.lock = NULL;
}
@@ -621,9 +620,7 @@ int SDL_StartEventLoop(void)
#endif
SDL_EventQ.active = SDL_TRUE;
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
+ SDL_UnlockMutex(SDL_EventQ.lock);
return 0;
}
@@ -717,17 +714,18 @@ static int SDL_SendWakeupEvent()
if (_this == NULL || !_this->SendWakeupEvent) {
return 0;
}
- if (!_this->wakeup_lock || SDL_LockMutex(_this->wakeup_lock) == 0) {
+
+ SDL_LockMutex(_this->wakeup_lock);
+ {
if (_this->wakeup_window) {
_this->SendWakeupEvent(_this, _this->wakeup_window);
/* No more wakeup events needed until we enter a new wait */
_this->wakeup_window = NULL;
}
- if (_this->wakeup_lock) {
- SDL_UnlockMutex(_this->wakeup_lock);
- }
}
+ SDL_UnlockMutex(_this->wakeup_lock);
+
return 0;
}
@@ -739,16 +737,16 @@ static int SDL_PeepEventsInternal(SDL_Event *events, int numevents, SDL_eventact
/* Lock the event queue */
used = 0;
- if (!SDL_EventQ.lock || SDL_LockMutex(SDL_EventQ.lock) == 0) {
+
+ SDL_LockMutex(SDL_EventQ.lock);
+ {
/* Don't look after we've quit */
if (!SDL_EventQ.active) {
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
/* We get a few spurious events at shutdown, so don't warn then */
if (action == SDL_GETEVENT) {
SDL_SetError("The event system has been shut down");
}
+ SDL_UnlockMutex(SDL_EventQ.lock);
return -1;
}
if (action == SDL_ADDEVENT) {
@@ -817,12 +815,8 @@ static int SDL_PeepEventsInternal(SDL_Event *events, int numevents, SDL_eventact
}
}
}
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
- } else {
- return SDL_SetError("Couldn't lock event queue");
}
+ SDL_UnlockMutex(SDL_EventQ.lock);
if (used > 0 && action == SDL_ADDEVENT) {
SDL_SendWakeupEvent();
@@ -836,14 +830,12 @@ int SDL_PeepEvents(SDL_Event *events, int numevents, SDL_eventaction action,
return SDL_PeepEventsInternal(events, numevents, action, minType, maxType, SDL_FALSE);
}
-SDL_bool
-SDL_HasEvent(Uint32 type)
+SDL_bool SDL_HasEvent(Uint32 type)
{
return SDL_PeepEvents(NULL, 0, SDL_PEEKEVENT, type, type) > 0;
}
-SDL_bool
-SDL_HasEvents(Uint32 minType, Uint32 maxType)
+SDL_bool SDL_HasEvents(Uint32 minType, Uint32 maxType)
{
return SDL_PeepEvents(NULL, 0, SDL_PEEKEVENT, minType, maxType) > 0;
}
@@ -870,12 +862,11 @@ void SDL_FlushEvents(Uint32 minType, Uint32 maxType)
#endif
/* Lock the event queue */
- if (!SDL_EventQ.lock || SDL_LockMutex(SDL_EventQ.lock) == 0) {
+ SDL_LockMutex(SDL_EventQ.lock);
+ {
/* Don't look after we've quit */
if (!SDL_EventQ.active) {
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
+ SDL_UnlockMutex(SDL_EventQ.lock);
return;
}
for (entry = SDL_EventQ.head; entry; entry = next) {
@@ -885,10 +876,8 @@ void SDL_FlushEvents(Uint32 minType, Uint32 maxType)
SDL_CutEvent(entry);
}
}
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
}
+ SDL_UnlockMutex(SDL_EventQ.lock);
}
/* Run the system dependent event loops */
@@ -964,6 +953,7 @@ static int SDL_WaitEventTimeout_Device(_THIS, SDL_Window *wakeup_window, SDL_Eve
SDL_bool need_periodic_poll = SDL_events_need_periodic_poll();
for (;;) {
+ int status;
/* Pump events on entry and each time we wake to ensure:
a) All pending events are batch processed after waking up from a wait
b) Waiting can be completely skipped if events are already available to be pumped
@@ -976,55 +966,55 @@ static int SDL_WaitEventTimeout_Device(_THIS, SDL_Window *wakeup_window, SDL_Eve
SDL_bool add_sentinel = (SDL_AtomicGet(&SDL_sentinel_pending) == 0) ? SDL_TRUE : SDL_FALSE;
SDL_PumpEventsInternal(add_sentinel);
- if (!_this->wakeup_lock || SDL_LockMutex(_this->wakeup_lock) == 0) {
- int status = SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT);
+ SDL_LockMutex(_this->wakeup_lock);
+ {
+ status = SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT);
/* If status == 0 we are going to block so wakeup will be needed. */
if (status == 0) {
_this->wakeup_window = wakeup_window;
} else {
_this->wakeup_window = NULL;
}
- if (_this->wakeup_lock) {
- SDL_UnlockMutex(_this->wakeup_lock);
- }
- if (status < 0) {
- /* Got an error: return */
- break;
- }
- if (status > 0) {
- /* There is an event, we can return. */
- return 1;
- }
- /* No events found in the queue, call WaitEventTimeout to wait for an event. */
- if (timeoutNS > 0) {
- Sint64 elapsed = SDL_GetTicksNS() - start;
- if (elapsed >= timeoutNS) {
- /* Set wakeup_window to NULL without holding the lock. */
- _this->wakeup_window = NULL;
- return 0;
- }
- loop_timeoutNS = (timeoutNS - elapsed);
- }
- if (need_periodic_poll) {
- if (loop_timeoutNS >= 0) {
- loop_timeoutNS = SDL_min(loop_timeoutNS, PERIODIC_POLL_INTERVAL_NS);
- } else {
- loop_timeoutNS = PERIODIC_POLL_INTERVAL_NS;
- }
+ }
+ SDL_UnlockMutex(_this->wakeup_lock);
+
+ if (status < 0) {
+ /* Got an error: return */
+ break;
+ }
+ if (status > 0) {
+ /* There is an event, we can return. */
+ return 1;
+ }
+ /* No events found in the queue, call WaitEventTimeout to wait for an event. */
+ if (timeoutNS > 0) {
+ Sint64 elapsed = SDL_GetTicksNS() - start;
+ if (elapsed >= timeoutNS) {
+ /* Set wakeup_window to NULL without holding the lock. */
+ _this->wakeup_window = NULL;
+ return 0;
}
- status = _this->WaitEventTimeout(_this, loop_timeoutNS);
- /* Set wakeup_window to NULL without holding the lock. */
- _this->wakeup_window = NULL;
- if (status == 0 && need_periodic_poll && loop_timeoutNS == PERIODIC_POLL_INTERVAL_NS) {
- /* We may have woken up to poll. Try again */
- continue;
- } else if (status <= 0) {
- /* There is either an error or the timeout is elapsed: return */
- return status;
+ loop_timeoutNS = (timeoutNS - elapsed);
+ }
+ if (need_periodic_poll) {
+ if (loop_timeoutNS >= 0) {
+ loop_timeoutNS = SDL_min(loop_timeoutNS, PERIODIC_POLL_INTERVAL_NS);
+ } else {
+ loop_timeoutNS = PERIODIC_POLL_INTERVAL_NS;
}
- /* An event was found and pumped into the SDL events queue. Continue the loop
- to let SDL_PeepEvents pick it up .*/
}
+ status = _this->WaitEventTimeout(_this, loop_timeoutNS);
+ /* Set wakeup_window to NULL without holding the lock. */
+ _this->wakeup_window = NULL;
+ if (status == 0 && need_periodic_poll && loop_timeoutNS == PERIODIC_POLL_INTERVAL_NS) {
+ /* We may have woken up to poll. Try again */
+ continue;
+ } else if (status <= 0) {
+ /* There is either an error or the timeout is elapsed: return */
+ return status;
+ }
+ /* An event was found and pumped into the SDL events queue. Continue the loop
+ to let SDL_PeepEvents pick it up .*/
}
return 0;
}
@@ -1172,11 +1162,10 @@ int SDL_PushEvent(SDL_Event *event)
}
if (SDL_EventOK.callback || SDL_event_watchers_count > 0) {
- if (SDL_event_watchers_lock == NULL || SDL_LockMutex(SDL_event_watchers_lock) == 0) {
+ SDL_LockMutex(SDL_event_watchers_lock);
+ {
if (SDL_EventOK.callback && !SDL_EventOK.callback(SDL_EventOK.userdata, event)) {
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
+ SDL_UnlockMutex(SDL_event_watchers_lock);
return 0;
}
@@ -1204,11 +1193,8 @@ int SDL_PushEvent(SDL_Event *event)
SDL_event_watchers_removed = SDL_FALSE;
}
}
-
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
}
+ SDL_UnlockMutex(SDL_event_watchers_lock);
}
if (SDL_PeepEvents(event, 1, SDL_ADDEVENT, 0, 0) <= 0) {
@@ -1220,32 +1206,25 @@ int SDL_PushEvent(SDL_Event *event)
void SDL_SetEventFilter(SDL_EventFilter filter, void *userdata)
{
- if (SDL_event_watchers_lock == NULL || SDL_LockMutex(SDL_event_watchers_lock) == 0) {
+ SDL_LockMutex(SDL_event_watchers_lock);
+ {
/* Set filter and discard pending events */
SDL_EventOK.callback = filter;
SDL_EventOK.userdata = userdata;
SDL_FlushEvents(SDL_FIRSTEVENT, SDL_LASTEVENT);
-
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
}
+ SDL_UnlockMutex(SDL_event_watchers_lock);
}
-SDL_bool
-SDL_GetEventFilter(SDL_EventFilter *filter, void **userdata)
+SDL_bool SDL_GetEventFilter(SDL_EventFilter *filter, void **userdata)
{
SDL_EventWatcher event_ok;
- if (SDL_event_watchers_lock == NULL || SDL_LockMutex(SDL_event_watchers_lock) == 0) {
+ SDL_LockMutex(SDL_event_watchers_lock);
+ {
event_ok = SDL_EventOK;
-
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
- } else {
- SDL_zero(event_ok);
}
+ SDL_UnlockMutex(SDL_event_watchers_lock);
if (filter) {
*filter = event_ok.callback;
@@ -1258,7 +1237,8 @@ SDL_GetEventFilter(SDL_EventFilter *filter, void **userdata)
void SDL_AddEventWatch(SDL_EventFilter filter, void *userdata)
{
- if (SDL_event_watchers_lock == NULL || SDL_LockMutex(SDL_event_watchers_lock) == 0) {
+ SDL_LockMutex(SDL_event_watchers_lock);
+ {
SDL_EventWatcher *event_watchers;
event_watchers = SDL_realloc(SDL_event_watchers, (SDL_event_watchers_count + 1) * sizeof(*event_watchers));
@@ -1272,16 +1252,14 @@ void SDL_AddEventWatch(SDL_EventFilter filter, void *userdata)
watcher->removed = SDL_FALSE;
++SDL_event_watchers_count;
}
-
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
}
+ SDL_UnlockMutex(SDL_event_watchers_lock);
}
void SDL_DelEventWatch(SDL_EventFilter filter, void *userdata)
{
- if (SDL_event_watchers_lock == NULL || SDL_LockMutex(SDL_event_watchers_lock) == 0) {
+ SDL_LockMutex(SDL_event_watchers_lock);
+ {
int i;
for (i = 0; i < SDL_event_watchers_count; ++i) {
@@ -1298,16 +1276,14 @@ void SDL_DelEventWatch(SDL_EventFilter filter, void *userdata)
break;
}
}
-
- if (SDL_event_watchers_lock) {
- SDL_UnlockMutex(SDL_event_watchers_lock);
- }
}
+ SDL_UnlockMutex(SDL_event_watchers_lock);
}
void SDL_FilterEvents(SDL_EventFilter filter, void *userdata)
{
- if (!SDL_EventQ.lock || SDL_LockMutex(SDL_EventQ.lock) == 0) {
+ SDL_LockMutex(SDL_EventQ.lock);
+ {
SDL_EventEntry *entry, *next;
for (entry = SDL_EventQ.head; entry; entry = next) {
next = entry->next;
@@ -1315,10 +1291,8 @@ void SDL_FilterEvents(SDL_EventFilter filter, void *userdata)
SDL_CutEvent(entry);
}
}
- if (SDL_EventQ.lock) {
- SDL_UnlockMutex(SDL_EventQ.lock);
- }
}
+ SDL_UnlockMutex(SDL_EventQ.lock);
}
Uint8 SDL_EventState(Uint32 type, int state)
diff --git a/src/haptic/SDL_haptic.c b/src/haptic/SDL_haptic.c
index 60d27535684c..69b8c14f5c4f 100644
--- a/src/haptic/SDL_haptic.c
+++ b/src/haptic/SDL_haptic.c
@@ -233,12 +233,17 @@ int SDL_JoystickIsHaptic(SDL_Joystick *joystick)
{
int ret;
- /* Must be a valid joystick */
- if (!SDL_PrivateJoystickValid(joystick)) {
- return -1;
- }
+ SDL_LockJoysticks();
+ {
+ /* Must be a valid joystick */
+ if (!SDL_PrivateJoystickValid(joystick)) {
+ SDL_UnlockJoysticks();
+ return -1;
+ }
- ret = SDL_SYS_JoystickIsHaptic(joystick);
+ ret = SDL_SYS_JoystickIsHaptic(joystick);
+ }
+ SDL_UnlockJoysticks();
if (ret > 0) {
return SDL_TRUE;
@@ -265,44 +270,53 @@ SDL_HapticOpenFromJoystick(SDL_Joystick *joystick)
return NULL;
}
- /* Must be a valid joystick */
- if (!SDL_PrivateJoystickValid(joystick)) {
- SDL_SetError("Haptic: Joystick isn't valid.");
- return NULL;
- }
+ SDL_LockJoysticks();
+ {
+ /* Must be a valid joystick */
+ if (!SDL_PrivateJoystickValid(joystick)) {
+ SDL_SetError("Haptic: Joystick isn't valid.");
+ SDL_UnlockJoysticks();
+ return NULL;
+ }
- /* Joystick must be haptic */
- if (SDL_SYS_JoystickIsHaptic(joystick) <= 0) {
- SDL_SetError("Haptic: Joystick isn't a haptic device.");
- return NULL;
- }
+ /* Joystick must be haptic */
+ if (SDL_SYS_JoystickIsHaptic(joystick) <= 0) {
+ SDL_SetError("Haptic: Joystick isn't a haptic device.");
+ SDL_UnlockJoysticks();
+ return NULL;
+ }
- hapticlist = SDL_haptics;
- /* Check to see if joystick's haptic is already open */
- while (hapticlist) {
- if (SDL_SYS_JoystickSameHaptic(hapticlist, joystick)) {
- haptic = hapticlist;
- ++haptic->ref_count;
- return haptic;
+ hapticlist = SDL_haptics;
+ /* Check to see if joystick's haptic is already open */
+ while (hapticlist) {
+ if (SDL_SYS_JoystickSameHaptic(hapticlist, joystick)) {
+ haptic = hapticlist;
+ ++haptic->ref_count;
+ SDL_UnlockJoysticks();
+ return haptic;
+ }
+ hapticlist = hapticlist->next;
}
- hapticlist = hapticlist->next;
- }
- /* Create the haptic device */
- haptic = (SDL_Haptic *)SDL_malloc((sizeof *haptic));
- if (haptic == NULL) {
- SDL_OutOfMemory();
- return NULL;
- }
+ /* Create the haptic device */
+ haptic = (SDL_Haptic *)SDL_malloc((sizeof *haptic));
+ if (haptic == NULL) {
+ SDL_OutOfMemory();
+ SDL_UnlockJoysticks();
+ return NULL;
+ }
- /* Initialize the haptic device */
- SDL_memset(haptic, 0, sizeof(SDL_Haptic));
- haptic->rumble_id = -1;
- if (SDL_SYS_HapticOpenFromJoystick(haptic, joystick) < 0) {
- SDL_SetError("Haptic: SDL_SYS_HapticOpenFromJoystick failed.");
- SDL_free(haptic);
- return NULL;
+ /* Initialize the haptic device */
+ SDL_memset(haptic, 0, sizeof(SDL_Haptic));
+ haptic->rumble_id = -1;
+ if (SDL_SYS_HapticOpenFromJoystick(haptic, joystick) < 0) {
+ SDL_SetError("Haptic: SDL_SYS_HapticOpenFromJoystick failed.");
+ SDL_free(haptic);
+ SDL_UnlockJoysticks();
+ return NULL;
+ }
}
+ SDL_UnlockJoysticks();
/* Add haptic to list */
++haptic->ref_count;
diff --git a/src/haptic/linux/SDL_syshaptic.c b/src/haptic/linux/SDL_syshaptic.c
index 54aa5c043e89..774f5a12f54e 100644
--- a/src/haptic/linux/SDL_syshaptic.c
+++ b/src/haptic/linux/SDL_syshaptic.c
@@ -481,6 +481,8 @@ int SDL_SYS_HapticMouse(void)
int SDL_SYS_JoystickIsHaptic(SDL_Joystick *joystick)
{
#ifdef SDL_JOYSTICK_LINUX
+ SDL_AssertJoysticksLocked();
+
if (joystick->driver != &SDL_LINUX_JoystickDriver) {
return SDL_FALSE;
}
@@ -497,6 +499,8 @@ int SDL_SYS_JoystickIsHaptic(SDL_Joystick *joystick)
int SDL_SYS_JoystickSameHaptic(SDL_Haptic *haptic, SDL_Joystick *joystick)
{
#ifdef SDL_JOYSTICK_LINUX
+ SDL_AssertJoysticksLocked();
+
if (joystick->driver != &SDL_LINUX_JoystickDriver) {
return 0;
}
@@ -520,6 +524,8 @@ int SDL_SYS_HapticOpenFromJoystick(SDL_Haptic *haptic, SDL_Joystick *joystick)
int ret;
SDL_hapticlist_item *item;
+ SDL_AssertJoysticksLocked();
+
if (joystick->driver != &SDL_LINUX_JoystickDriver) {
return -1;
}
diff --git a/src/joystick/SDL_gamecontroller.c b/src/joystick/SDL_gamecontroller.c
index b54d7591aa61..fa9c600f00cd 100644
--- a/src/joystick/SDL_gamecontroller.c
+++ b/src/joystick/SDL_gamecontroller.c
@@ -51,7 +51,7 @@
#define SDL_CONTROLLER_SDKLE_FIELD_SIZE SDL_strlen(SDL_CONTROLLER_SDKLE_FIELD)
/* a list of currently opened game controllers */
-static SDL_GameController *SDL_gamecontrollers = NULL;
+static SDL_GameController *SDL_gamecontrollers SDL_GUARDED_BY(SDL_joystick_lock) = NULL;
typedef struct
{
@@ -99,44 +99,53 @@ typedef enum
SDL_CONTROLLER_MAPPING_PRIORITY_USER,
} SDL_ControllerMappingPriority;
+#define _guarded SDL_GUARDED_BY(SDL_joystick_lock)
+
typedef struct _ControllerMapping_t
{
- SDL_JoystickGUID guid;
- char *name;
- char *mapping;
- SDL_ControllerMappingPriority priority;
- struct _ControllerMapping_t *next;
+ SDL_JoystickGUID guid _guarded;
+ char *name _guarded;
+ char *mapping _guarded;
+ SDL_ControllerMappingPriority priority _guarded;
+ struct _ControllerMapping_t *next _guarded;
} ControllerMapping_t;
+#undef _guarded
+
static SDL_JoystickGUID s_zeroGUID;
-static ControllerMapping_t *s_pSupportedControllers = NULL;
-static ControllerMapping_t *s_pDefaultMapping = NULL;
-static ControllerMapping_t *s_pXInputMapping = NULL;
+static ControllerMapping_t *s_pSupportedControllers SDL_GUARDED_BY(SDL_joystick_lock) = NULL;
+static ControllerMapping_t *s_pDefaultMapping SDL_GUARDED_BY(SDL_joystick_lock) = NULL;
+static ControllerMapping_t *s_pXInputMapping SDL_GUARDED_BY(SDL_joystick_lock) = NULL;
static char gamecontroller_magic;
+#define _guarded SDL_GUARDED_BY(SDL_joystick_lock)
+
/* The SDL game controller structure */
struct _SDL_GameController
{
- const void *magic;
+ const void *magic _guarded;
- SDL_Joystick *joystick; /* underlying joystick device */
- int ref_count;
+ SDL_Joystick *joystick _guarded; /* underlying joystick device */
+ int ref_count _guarded;
- const char *name;
- ControllerMapping_t *mapping;
- int num_bindings;
- SDL_ExtendedGameControllerBind *bindings;
- SDL_ExtendedGameControllerBind **last_match_axis;
- Uint8 *last_hat_mask;
- Uint64 guide_button_down;
+ const char *name _guarded;
+ ControllerMapping_t *mapping _guarded;
+ int num_bindings _guarded;
+ SDL_ExtendedGameControllerBind *bindings _guarded;
+ SDL_ExtendedGameControllerBind **last_match_axis _guarded;
+ Uint8 *last_hat_mask _guarded;
+ Uint64 guide_button_down _guarded;
- struct _SDL_GameController *next; /* pointer to next
(Patch may be truncated, please check the link at the top of this post.)