eden-emu/SDL
19
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Added support for clang thread-safety analysis

Browse source 
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"
This commit is contained in:
Sam Lantinga 2022-12-13 14:03:40 -08:00
parent 582fb3901a
commit d59caffe2c
42 changed files with 1667 additions and 1174 deletions
Download patch file Download diff file Expand all files Collapse all files

44
src/joystick/hidapi/SDL_hidapijoystick.c
View file

@ -91,7 +91,7 @@ static int SDL_HIDAPI_numdrivers = 0;
static SDL_SpinLock SDL_HIDAPI_spinlock;
static SDL_bool SDL_HIDAPI_hints_changed = SDL_FALSE;
static Uint32 SDL_HIDAPI_change_count = 0;
static SDL_HIDAPI_Device *SDL_HIDAPI_devices;
static SDL_HIDAPI_Device *SDL_HIDAPI_devices SDL_GUARDED_BY(SDL_joystick_lock);
static int SDL_HIDAPI_numjoysticks = 0;
static SDL_bool SDL_HIDAPI_combine_joycons = SDL_TRUE;
static SDL_bool initialized = SDL_FALSE;
@ -264,6 +264,8 @@ static SDL_HIDAPI_Device *HIDAPI_GetDeviceByIndex(int device_index, SDL_Joystick
{
SDL_HIDAPI_Device *device;
SDL_AssertJoysticksLocked();
for (device = SDL_HIDAPI_devices; device; device = device->next) {
if (device->parent) {
continue;
@ -285,6 +287,8 @@ static SDL_HIDAPI_Device *HIDAPI_GetJoystickByInfo(const char *path, Uint16 vend
{
SDL_HIDAPI_Device *device;
SDL_AssertJoysticksLocked();
for (device = SDL_HIDAPI_devices; device; device = device->next) {
if (device->vendor_id == vendor_id && device->product_id == product_id &&
SDL_strcmp(device->path, path) == 0) {
@ -323,7 +327,7 @@ static void HIDAPI_CleanupDeviceDriver(SDL_HIDAPI_Device *device)
SDL_UnlockMutex(device->dev_lock);
}
static void HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device, SDL_bool *removed)
static void HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device, SDL_bool *removed) SDL_NO_THREAD_SAFETY_ANALYSIS /* We unlock the joystick lock to be able to open the HID device on Android */
{
*removed = SDL_FALSE;
@ -426,6 +430,8 @@ static void SDL_HIDAPI_UpdateDrivers(void)
SDL_HIDAPI_Device *device;
SDL_bool removed;
SDL_AssertJoysticksLocked();
SDL_HIDAPI_numdrivers = 0;
for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
@ -571,6 +577,8 @@ HIDAPI_HasConnectedUSBDevice(const char *serial)
{
SDL_HIDAPI_Device *device;
SDL_AssertJoysticksLocked();
if (serial == NULL) {
return SDL_FALSE;
}
@ -595,6 +603,8 @@ void HIDAPI_DisconnectBluetoothDevice(const char *serial)
{
SDL_HIDAPI_Device *device;
SDL_AssertJoysticksLocked();
if (serial == NULL) {
return;
}
@ -622,6 +632,8 @@ HIDAPI_JoystickConnected(SDL_HIDAPI_Device *device, SDL_JoystickID *pJoystickID)
int i, j;
SDL_JoystickID joystickID;
SDL_AssertJoysticksLocked();
for (i = 0; i < device->num_children; ++i) {
SDL_HIDAPI_Device *child = device->children[i];
for (j = child->num_joysticks; j--;) {
@ -717,6 +729,8 @@ static SDL_HIDAPI_Device *HIDAPI_AddDevice(const struct SDL_hid_device_info *inf
SDL_HIDAPI_Device *curr, *last = NULL;
SDL_bool removed;
SDL_AssertJoysticksLocked();
for (curr = SDL_HIDAPI_devices, last = NULL; curr; last = curr, curr = curr->next) {
}
@ -810,6 +824,8 @@ static void HIDAPI_DelDevice(SDL_HIDAPI_Device *device)
SDL_HIDAPI_Device *curr, *last;
int i;
SDL_AssertJoysticksLocked();
#ifdef DEBUG_HIDAPI
SDL_Log("Removing HIDAPI device '%s' VID 0x%.4x, PID 0x%.4x, version %d, serial %s, interface %d, interface_class %d, interface_subclass %d, interface_protocol %d, usage page 0x%.4x, usage 0x%.4x, path = %s, driver = %s (%s)\n", device->name, device->vendor_id, device->product_id, device->version, device->serial ? device->serial : "NONE", device->interface_number, device->interface_class, device->interface_subclass, device->interface_protocol, device->usage_page, device->usage, device->path, device->driver ? device->driver->name : "NONE", device->driver && device->driver->enabled ? "ENABLED" : "DISABLED");
#endif
@ -849,6 +865,8 @@ static SDL_bool HIDAPI_CreateCombinedJoyCons()
SDL_HIDAPI_Device *device, *combined;
SDL_HIDAPI_Device *joycons[2] = { NULL, NULL };
SDL_AssertJoysticksLocked();
if (!SDL_HIDAPI_combine_joycons) {
return SDL_FALSE;
}
@ -1160,6 +1178,8 @@ void HIDAPI_UpdateDevices(void)
{
SDL_HIDAPI_Device *device;
SDL_AssertJoysticksLocked();
/* Update the devices, which may change connected joysticks and send events */
/* Prepare the existing device list */
@ -1262,6 +1282,8 @@ static int HIDAPI_JoystickOpen(SDL_Joystick *joystick, int device_index)
SDL_HIDAPI_Device *device = HIDAPI_GetDeviceByIndex(device_index, &joystickID);
struct joystick_hwdata *hwdata;
SDL_AssertJoysticksLocked();
if (device == NULL || !device->driver) {
/* This should never happen - validated before being called */
return SDL_SetError("Couldn't find HIDAPI device at index %d\n", device_index);
@ -1299,6 +1321,8 @@ static int HIDAPI_JoystickRumble(SDL_Joystick *joystick, Uint16 low_frequency_ru
{
int result;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1314,6 +1338,8 @@ static int HIDAPI_JoystickRumbleTriggers(SDL_Joystick *joystick, Uint16 left_rum
{
int result;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1329,6 +1355,8 @@ static Uint32 HIDAPI_JoystickGetCapabilities(SDL_Joystick *joystick)
{
Uint32 result = 0;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1342,6 +1370,8 @@ static int HIDAPI_JoystickSetLED(SDL_Joystick *joystick, Uint8 red, Uint8 green,
{
int result;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1357,6 +1387,8 @@ static int HIDAPI_JoystickSendEffect(SDL_Joystick *joystick, const void *data, i
{
int result;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1372,6 +1404,8 @@ static int HIDAPI_JoystickSetSensorsEnabled(SDL_Joystick *joystick, SDL_bool ena
{
int result;
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1388,8 +1422,10 @@ static void HIDAPI_JoystickUpdate(SDL_Joystick *joystick)
/* This is handled in SDL_HIDAPI_UpdateDevices() */
}
static void HIDAPI_JoystickClose(SDL_Joystick *joystick)
static void HIDAPI_JoystickClose(SDL_Joystick *joystick) SDL_NO_THREAD_SAFETY_ANALYSIS /* We unlock the device lock so rumble can complete */
{
SDL_AssertJoysticksLocked();
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
int i;
@ -1420,6 +1456,8 @@ static void HIDAPI_JoystickQuit(void)
{
int i;
SDL_AssertJoysticksLocked();
shutting_down = SDL_TRUE;
SDL_HIDAPI_QuitRumble();