Fixed deadlock in HIDAPI joystick system

Sam Lantinga 2020-01-13 22:05:54 -08:00
parent 3a796d6a58
commit 7775f7cedf
2 changed files with 40 additions and 40 deletions

View File

@ -751,10 +751,17 @@ SDL_JoystickSetPlayerIndex(SDL_Joystick * joystick, int player_index)
int int
SDL_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint16 high_frequency_rumble, Uint32 duration_ms) SDL_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint16 high_frequency_rumble, Uint32 duration_ms)
{ {
int result;
if (!SDL_PrivateJoystickValid(joystick)) { if (!SDL_PrivateJoystickValid(joystick)) {
return -1; return -1;
} }
return joystick->driver->Rumble(joystick, low_frequency_rumble, high_frequency_rumble, duration_ms);
SDL_LockJoysticks();
result = joystick->driver->Rumble(joystick, low_frequency_rumble, high_frequency_rumble, duration_ms);
SDL_UnlockJoysticks();
return result;
} }
/* /*

View File

@ -23,10 +23,10 @@
#ifdef SDL_JOYSTICK_HIDAPI #ifdef SDL_JOYSTICK_HIDAPI
#include "SDL_assert.h" #include "SDL_assert.h"
#include "SDL_atomic.h"
#include "SDL_endian.h" #include "SDL_endian.h"
#include "SDL_hints.h" #include "SDL_hints.h"
#include "SDL_log.h" #include "SDL_log.h"
#include "SDL_mutex.h"
#include "SDL_thread.h" #include "SDL_thread.h"
#include "SDL_timer.h" #include "SDL_timer.h"
#include "SDL_joystick.h" #include "SDL_joystick.h"
@ -79,7 +79,7 @@ static SDL_HIDAPI_DeviceDriver *SDL_HIDAPI_drivers[] = {
#endif #endif
}; };
static int SDL_HIDAPI_numdrivers = 0; static int SDL_HIDAPI_numdrivers = 0;
static SDL_mutex *SDL_HIDAPI_mutex; static SDL_SpinLock SDL_HIDAPI_spinlock;
static SDL_HIDAPI_Device *SDL_HIDAPI_devices; static SDL_HIDAPI_Device *SDL_HIDAPI_devices;
static int SDL_HIDAPI_numjoysticks = 0; static int SDL_HIDAPI_numjoysticks = 0;
static SDL_bool initialized = SDL_FALSE; static SDL_bool initialized = SDL_FALSE;
@ -529,7 +529,7 @@ SDL_HIDAPIDriverHintChanged(void *userdata, const char *name, const char *oldVal
} }
/* Update device list if driver availability changes */ /* Update device list if driver availability changes */
SDL_LockMutex(SDL_HIDAPI_mutex); SDL_LockJoysticks();
while (device) { while (device) {
if (device->driver && !device->driver->enabled) { if (device->driver && !device->driver->enabled) {
@ -539,7 +539,7 @@ SDL_HIDAPIDriverHintChanged(void *userdata, const char *name, const char *oldVal
device = device->next; device = device->next;
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex); SDL_UnlockJoysticks();
} }
static int static int
@ -556,8 +556,6 @@ HIDAPI_JoystickInit(void)
return -1; return -1;
} }
SDL_HIDAPI_mutex = SDL_CreateMutex();
for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) { for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i]; SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
SDL_AddHintCallback(driver->hint, SDL_HIDAPIDriverHintChanged, NULL); SDL_AddHintCallback(driver->hint, SDL_HIDAPIDriverHintChanged, NULL);
@ -771,7 +769,7 @@ HIDAPI_UpdateDeviceList(void)
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
struct hid_device_info *devs, *info; struct hid_device_info *devs, *info;
SDL_LockMutex(SDL_HIDAPI_mutex); SDL_LockJoysticks();
/* Prepare the existing device list */ /* Prepare the existing device list */
device = SDL_HIDAPI_devices; device = SDL_HIDAPI_devices;
@ -807,13 +805,14 @@ HIDAPI_UpdateDeviceList(void)
device = next; device = next;
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex); SDL_UnlockJoysticks();
} }
SDL_bool SDL_bool
HIDAPI_IsDevicePresent(Uint16 vendor_id, Uint16 product_id, Uint16 version, const char *name) HIDAPI_IsDevicePresent(Uint16 vendor_id, Uint16 product_id, Uint16 version, const char *name)
{ {
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
SDL_bool result = SDL_FALSE;
/* Make sure we're initialized, as this could be called from other drivers during startup */ /* Make sure we're initialized, as this could be called from other drivers during startup */
if (HIDAPI_JoystickInit() < 0) { if (HIDAPI_JoystickInit() < 0) {
@ -826,30 +825,39 @@ HIDAPI_IsDevicePresent(Uint16 vendor_id, Uint16 product_id, Uint16 version, cons
} }
/* Make sure the device list is completely up to date when we check for device presence */ /* Make sure the device list is completely up to date when we check for device presence */
HIDAPI_UpdateDeviceList(); if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
HIDAPI_UpdateDeviceList();
SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
}
/* Note that this isn't a perfect check - there may be multiple devices with 0 VID/PID, /* Note that this isn't a perfect check - there may be multiple devices with 0 VID/PID,
or a different name than we have it listed here, etc, but if we support the device or a different name than we have it listed here, etc, but if we support the device
and we have something similar in our device list, mark it as present. and we have something similar in our device list, mark it as present.
*/ */
SDL_LockJoysticks();
device = SDL_HIDAPI_devices; device = SDL_HIDAPI_devices;
while (device) { while (device) {
if (device->vendor_id == vendor_id && device->product_id == product_id && device->driver) { if (device->vendor_id == vendor_id && device->product_id == product_id && device->driver) {
return SDL_TRUE; result = SDL_TRUE;
} }
device = device->next; device = device->next;
} }
return SDL_FALSE; SDL_UnlockJoysticks();
return result;
} }
static void static void
HIDAPI_JoystickDetect(void) HIDAPI_JoystickDetect(void)
{ {
HIDAPI_UpdateDiscovery(); if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
if (SDL_HIDAPI_discovery.m_bHaveDevicesChanged) { HIDAPI_UpdateDiscovery();
/* FIXME: We probably need to schedule an update in a few seconds as well */ if (SDL_HIDAPI_discovery.m_bHaveDevicesChanged) {
HIDAPI_UpdateDeviceList(); /* FIXME: We probably need to schedule an update in a few seconds as well */
SDL_HIDAPI_discovery.m_bHaveDevicesChanged = SDL_FALSE; HIDAPI_UpdateDeviceList();
SDL_HIDAPI_discovery.m_bHaveDevicesChanged = SDL_FALSE;
}
SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
} }
} }
@ -859,18 +867,18 @@ HIDAPI_UpdateDevices(void)
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
/* Update the devices, which may change connected joysticks and send events */ /* Update the devices, which may change connected joysticks and send events */
SDL_LockMutex(SDL_HIDAPI_mutex);
/* Prepare the existing device list */ /* Prepare the existing device list */
device = SDL_HIDAPI_devices; if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
while (device) { device = SDL_HIDAPI_devices;
if (device->driver) { while (device) {
device->driver->UpdateDevice(device); if (device->driver) {
device->driver->UpdateDevice(device);
}
device = device->next;
} }
device = device->next; SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
} }
static const char * static const char *
@ -879,13 +887,11 @@ HIDAPI_JoystickGetDeviceName(int device_index)
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
const char *name = NULL; const char *name = NULL;
SDL_LockMutex(SDL_HIDAPI_mutex);
device = HIDAPI_GetDeviceByIndex(device_index, NULL); device = HIDAPI_GetDeviceByIndex(device_index, NULL);
if (device) { if (device) {
/* FIXME: The device could be freed after this name is returned... */ /* FIXME: The device could be freed after this name is returned... */
name = device->name; name = device->name;
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
return name; return name;
} }
@ -897,12 +903,10 @@ HIDAPI_JoystickGetDevicePlayerIndex(int device_index)
SDL_JoystickID instance_id; SDL_JoystickID instance_id;
int player_index = -1; int player_index = -1;
SDL_LockMutex(SDL_HIDAPI_mutex);
device = HIDAPI_GetDeviceByIndex(device_index, &instance_id); device = HIDAPI_GetDeviceByIndex(device_index, &instance_id);
if (device) { if (device) {
player_index = device->driver->GetDevicePlayerIndex(device, instance_id); player_index = device->driver->GetDevicePlayerIndex(device, instance_id);
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
return player_index; return player_index;
} }
@ -913,12 +917,10 @@ HIDAPI_JoystickSetDevicePlayerIndex(int device_index, int player_index)
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
SDL_JoystickID instance_id; SDL_JoystickID instance_id;
SDL_LockMutex(SDL_HIDAPI_mutex);
device = HIDAPI_GetDeviceByIndex(device_index, &instance_id); device = HIDAPI_GetDeviceByIndex(device_index, &instance_id);
if (device) { if (device) {
device->driver->SetDevicePlayerIndex(device, instance_id, player_index); device->driver->SetDevicePlayerIndex(device, instance_id, player_index);
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
} }
static SDL_JoystickGUID static SDL_JoystickGUID
@ -927,14 +929,12 @@ HIDAPI_JoystickGetDeviceGUID(int device_index)
SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *device;
SDL_JoystickGUID guid; SDL_JoystickGUID guid;
SDL_LockMutex(SDL_HIDAPI_mutex);
device = HIDAPI_GetDeviceByIndex(device_index, NULL); device = HIDAPI_GetDeviceByIndex(device_index, NULL);
if (device) { if (device) {
SDL_memcpy(&guid, &device->guid, sizeof(guid)); SDL_memcpy(&guid, &device->guid, sizeof(guid));
} else { } else {
SDL_zero(guid); SDL_zero(guid);
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
return guid; return guid;
} }
@ -943,9 +943,7 @@ static SDL_JoystickID
HIDAPI_JoystickGetDeviceInstanceID(int device_index) HIDAPI_JoystickGetDeviceInstanceID(int device_index)
{ {
SDL_JoystickID joystickID = -1; SDL_JoystickID joystickID = -1;
SDL_LockMutex(SDL_HIDAPI_mutex);
HIDAPI_GetDeviceByIndex(device_index, &joystickID); HIDAPI_GetDeviceByIndex(device_index, &joystickID);
SDL_UnlockMutex(SDL_HIDAPI_mutex);
return joystickID; return joystickID;
} }
@ -976,7 +974,6 @@ HIDAPI_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint
{ {
int result; int result;
SDL_LockMutex(SDL_HIDAPI_mutex);
if (joystick->hwdata) { if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device; SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -985,7 +982,6 @@ HIDAPI_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint
SDL_SetError("Rumble failed, device disconnected"); SDL_SetError("Rumble failed, device disconnected");
result = -1; result = -1;
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
return result; return result;
} }
@ -999,7 +995,6 @@ HIDAPI_JoystickUpdate(SDL_Joystick * joystick)
static void static void
HIDAPI_JoystickClose(SDL_Joystick * joystick) HIDAPI_JoystickClose(SDL_Joystick * joystick)
{ {
SDL_LockMutex(SDL_HIDAPI_mutex);
if (joystick->hwdata) { if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device; SDL_HIDAPI_Device *device = joystick->hwdata->device;
@ -1008,7 +1003,6 @@ HIDAPI_JoystickClose(SDL_Joystick * joystick)
SDL_free(joystick->hwdata); SDL_free(joystick->hwdata);
joystick->hwdata = NULL; joystick->hwdata = NULL;
} }
SDL_UnlockMutex(SDL_HIDAPI_mutex);
} }
static void static void
@ -1029,7 +1023,6 @@ HIDAPI_JoystickQuit(void)
} }
SDL_DelHintCallback(SDL_HINT_JOYSTICK_HIDAPI, SDL_DelHintCallback(SDL_HINT_JOYSTICK_HIDAPI,
SDL_HIDAPIDriverHintChanged, NULL); SDL_HIDAPIDriverHintChanged, NULL);
SDL_DestroyMutex(SDL_HIDAPI_mutex);
hid_exit(); hid_exit();