From d59caffe2c8e80aa9d38574fd17974ec5cf05d2b 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/SDL_joystick.h | 8 +-
include/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 | 191 ++--
src/haptic/SDL_haptic.c | 88 +-
src/haptic/linux/SDL_syshaptic.c | 6 +
src/joystick/SDL_gamecontroller.c | 1035 ++++++++++++--------
src/joystick/SDL_joystick.c | 826 +++++++++-------
src/joystick/SDL_joystick_c.h | 2 +-
src/joystick/SDL_sysjoystick.h | 82 +-
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 | 35 +-
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 | 37 +-
src/thread/os2/SDL_sysmutex.c | 10 +-
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 +-
42 files changed, 1657 insertions(+), 1164 deletions(-)
diff --git a/include/SDL_joystick.h b/include/SDL_joystick.h
index 72c6604df2a3..9dcadc7b4469 100644
--- a/include/SDL_joystick.h
+++ b/include/SDL_joystick.h
@@ -44,6 +44,7 @@
#include "SDL_stdinc.h"
#include "SDL_error.h"
#include "SDL_guid.h"
+#include "SDL_mutex.h"
#include "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 2.0.7.
*/
-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 2.0.7.
*/
-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/SDL_mutex.h b/include/SDL_mutex.h
index d4d55a4bf5f0..1e2f2f44dd7f 100644
--- a/include/SDL_mutex.h
+++ b/include/SDL_mutex.h
@@ -31,6 +31,80 @@
#include "SDL_stdinc.h"
#include "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 "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 2.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 2.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 a689a550b940..7ff864a01194 100644
--- a/src/SDL_assert.c
+++ b/src/SDL_assert.c
@@ -345,10 +345,8 @@ SDL_ReportAssertion(SDL_assert_data *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 e9199cc2e652..80d36b5a4665 100644
--- a/src/SDL_log.c
+++ b/src/SDL_log.c
@@ -336,15 +336,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 959ce0d7a162..251391a63c97 100644
--- a/src/audio/SDL_audio.c
+++ b/src/audio/SDL_audio.c
@@ -301,14 +301,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 7543f2a6b55a..adb8bec59e5e 100644
--- a/src/dynapi/SDL_dynapi.h
+++ b/src/dynapi/SDL_dynapi.h
@@ -59,7 +59,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 441038ef2b9b..39a7fe027570 100644
--- a/src/events/SDL_events.c
+++ b/src/events/SDL_events.c
@@ -546,9 +546,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;
@@ -605,8 +603,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;
}
@@ -650,9 +649,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;
}
@@ -746,17 +743,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;
}
@@ -768,16 +766,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) {
@@ -846,12 +844,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();
@@ -865,14 +859,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;
}
@@ -899,12 +891,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) {
@@ -914,10 +905,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 */
@@ -1002,58 +991,59 @@ static int SDL_WaitEventTimeout_Device(_THIS, SDL_Window *wakeup_window, SDL_Eve
/* We only want a single sentinel in the queue. We could get more than one if event is NULL,
* since the SDL_PeepEvents() call below won't remove it in that case.
*/
+ int status;
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 (timeout > 0) {
- Uint32 elapsed = SDL_GetTicks() - start;
- if (elapsed >= (Uint32)timeout) {
- /* Set wakeup_window to NULL without holding the lock. */
- _this->wakeup_window = NULL;
- return 0;
- }
- loop_timeout = (int)((Uint32)timeout - elapsed);
- }
- if (need_periodic_poll) {
- if (loop_timeout >= 0) {
- loop_timeout = SDL_min(loop_timeout, PERIODIC_POLL_INTERVAL_MS);
- } else {
- loop_timeout = PERIODIC_POLL_INTERVAL_MS;
- }
+ }
+ 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 (timeout > 0) {
+ Uint32 elapsed = SDL_GetTicks() - start;
+ if (elapsed >= (Uint32)timeout) {
+ /* Set wakeup_window to NULL without holding the lock. */
+ _this->wakeup_window = NULL;
+ return 0;
}
- status = _this->WaitEventTimeout(_this, loop_timeout);
- /* Set wakeup_window to NULL without holding the lock. */
- _this->wakeup_window = NULL;
- if (status == 0 && need_periodic_poll && loop_timeout == PERIODIC_POLL_INTERVAL_MS) {
- /* 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_timeout = (int)((Uint32)timeout - elapsed);
+ }
+ if (need_periodic_poll) {
+ if (loop_timeout >= 0) {
+ loop_timeout = SDL_min(loop_timeout, PERIODIC_POLL_INTERVAL_MS);
+ } else {
+ loop_timeout = PERIODIC_POLL_INTERVAL_MS;
}
- /* 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_timeout);
+ /* Set wakeup_window to NULL without holding the lock. */
+ _this->wakeup_window = NULL;
+ if (status == 0 && need_periodic_poll && loop_timeout == PERIODIC_POLL_INTERVAL_MS) {
+ /* 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;
}
@@ -1187,11 +1177,10 @@ int SDL_PushEvent(SDL_Event *event)
event->common.timestamp = SDL_GetTicks();
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;
}
@@ -1219,11 +1208,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) {
@@ -1237,32 +1223,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;
@@ -1275,7 +1254,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));
@@ -1289,16 +1269,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) {
@@ -1315,16 +1293,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;
@@ -1332,10 +1308,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)
@@ -1384,8 +1358,7 @@ Uint8 SDL_EventState(Uint32 type, int state)
return current_state;
}
-Uint32
-SDL_RegisterEvents(int numevents)
+Uint32 SDL_RegisterEvents(int numevents)
{
Uint32 event_base;
diff --git a/src/haptic/SDL_haptic.c b/src/haptic/SDL_haptic.c
index c20962cae399..f1035012d42b 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 1a333ec7fee4..721cff6fd15d 100644
--- a/src/haptic/linux/SDL_syshaptic.c
+++ b/src/haptic/linux/SDL_syshaptic.c
@@ -489,6 +489,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;
}
@@ -505,6 +507,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;
}
@@ -528,6 +532,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 4e0aa174f62b..f63368e0ddca 100644
--- a/src/joystick/SDL_gamecontroller.c
+++ b/src/joystick/SDL_gamecontroller.c
@@ -55,7 +55,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
{
@@ -103,44 +103,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;
- Uint32 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;
+ Uint32 guide_button_down _guarded;
- struct _SDL_GameController *next; /* pointer to next gam
(Patch may be truncated, please check the link at the top of this post.)