Fixed deadlock in HIDAPI joystick system
authorSam Lantinga <slouken@libsdl.org>
Mon, 13 Jan 2020 22:05:54 -0800
changeset 13413e025090ec0f9
parent 13412 4d575a1ef70a
child 13414 c6aa63b96647
Fixed deadlock in HIDAPI joystick system
src/joystick/SDL_joystick.c
src/joystick/hidapi/SDL_hidapijoystick.c
     1.1 --- a/src/joystick/SDL_joystick.c	Mon Jan 13 15:35:54 2020 -0800
     1.2 +++ b/src/joystick/SDL_joystick.c	Mon Jan 13 22:05:54 2020 -0800
     1.3 @@ -751,10 +751,17 @@
     1.4  int
     1.5  SDL_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint16 high_frequency_rumble, Uint32 duration_ms)
     1.6  {
     1.7 +	int result;
     1.8 +
     1.9      if (!SDL_PrivateJoystickValid(joystick)) {
    1.10          return -1;
    1.11      }
    1.12 -    return joystick->driver->Rumble(joystick, low_frequency_rumble, high_frequency_rumble, duration_ms);
    1.13 +
    1.14 +	SDL_LockJoysticks();
    1.15 +    result = joystick->driver->Rumble(joystick, low_frequency_rumble, high_frequency_rumble, duration_ms);
    1.16 +	SDL_UnlockJoysticks();
    1.17 +
    1.18 +	return result;
    1.19  }
    1.20  
    1.21  /*
     2.1 --- a/src/joystick/hidapi/SDL_hidapijoystick.c	Mon Jan 13 15:35:54 2020 -0800
     2.2 +++ b/src/joystick/hidapi/SDL_hidapijoystick.c	Mon Jan 13 22:05:54 2020 -0800
     2.3 @@ -23,10 +23,10 @@
     2.4  #ifdef SDL_JOYSTICK_HIDAPI
     2.5  
     2.6  #include "SDL_assert.h"
     2.7 +#include "SDL_atomic.h"
     2.8  #include "SDL_endian.h"
     2.9  #include "SDL_hints.h"
    2.10  #include "SDL_log.h"
    2.11 -#include "SDL_mutex.h"
    2.12  #include "SDL_thread.h"
    2.13  #include "SDL_timer.h"
    2.14  #include "SDL_joystick.h"
    2.15 @@ -79,7 +79,7 @@
    2.16  #endif
    2.17  };
    2.18  static int SDL_HIDAPI_numdrivers = 0;
    2.19 -static SDL_mutex *SDL_HIDAPI_mutex;
    2.20 +static SDL_SpinLock SDL_HIDAPI_spinlock;
    2.21  static SDL_HIDAPI_Device *SDL_HIDAPI_devices;
    2.22  static int SDL_HIDAPI_numjoysticks = 0;
    2.23  static SDL_bool initialized = SDL_FALSE;
    2.24 @@ -529,7 +529,7 @@
    2.25      }
    2.26  
    2.27      /* Update device list if driver availability changes */
    2.28 -    SDL_LockMutex(SDL_HIDAPI_mutex);
    2.29 +    SDL_LockJoysticks();
    2.30  
    2.31      while (device) {
    2.32          if (device->driver && !device->driver->enabled) {
    2.33 @@ -539,7 +539,7 @@
    2.34          device = device->next;
    2.35      }
    2.36  
    2.37 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
    2.38 +    SDL_UnlockJoysticks();
    2.39  }
    2.40  
    2.41  static int
    2.42 @@ -556,8 +556,6 @@
    2.43          return -1;
    2.44      }
    2.45  
    2.46 -    SDL_HIDAPI_mutex = SDL_CreateMutex();
    2.47 -
    2.48      for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
    2.49          SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
    2.50          SDL_AddHintCallback(driver->hint, SDL_HIDAPIDriverHintChanged, NULL);
    2.51 @@ -771,7 +769,7 @@
    2.52      SDL_HIDAPI_Device *device;
    2.53      struct hid_device_info *devs, *info;
    2.54  
    2.55 -    SDL_LockMutex(SDL_HIDAPI_mutex);
    2.56 +    SDL_LockJoysticks();
    2.57  
    2.58      /* Prepare the existing device list */
    2.59      device = SDL_HIDAPI_devices;
    2.60 @@ -807,13 +805,14 @@
    2.61          device = next;
    2.62      }
    2.63  
    2.64 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
    2.65 +    SDL_UnlockJoysticks();
    2.66  }
    2.67  
    2.68  SDL_bool
    2.69  HIDAPI_IsDevicePresent(Uint16 vendor_id, Uint16 product_id, Uint16 version, const char *name)
    2.70  {
    2.71      SDL_HIDAPI_Device *device;
    2.72 +    SDL_bool result = SDL_FALSE;
    2.73  
    2.74      /* Make sure we're initialized, as this could be called from other drivers during startup */
    2.75      if (HIDAPI_JoystickInit() < 0) {
    2.76 @@ -826,30 +825,39 @@
    2.77      }
    2.78  
    2.79      /* Make sure the device list is completely up to date when we check for device presence */
    2.80 -    HIDAPI_UpdateDeviceList();
    2.81 +    if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
    2.82 +        HIDAPI_UpdateDeviceList();
    2.83 +        SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
    2.84 +    }
    2.85  
    2.86      /* Note that this isn't a perfect check - there may be multiple devices with 0 VID/PID,
    2.87         or a different name than we have it listed here, etc, but if we support the device
    2.88         and we have something similar in our device list, mark it as present.
    2.89       */
    2.90 +    SDL_LockJoysticks();
    2.91      device = SDL_HIDAPI_devices;
    2.92      while (device) {
    2.93          if (device->vendor_id == vendor_id && device->product_id == product_id && device->driver) {
    2.94 -            return SDL_TRUE;
    2.95 +            result = SDL_TRUE;
    2.96          }
    2.97          device = device->next;
    2.98      }
    2.99 -    return SDL_FALSE;
   2.100 +    SDL_UnlockJoysticks();
   2.101 +
   2.102 +    return result;
   2.103  }
   2.104  
   2.105  static void
   2.106  HIDAPI_JoystickDetect(void)
   2.107  {
   2.108 -    HIDAPI_UpdateDiscovery();
   2.109 -    if (SDL_HIDAPI_discovery.m_bHaveDevicesChanged) {
   2.110 -        /* FIXME: We probably need to schedule an update in a few seconds as well */
   2.111 -        HIDAPI_UpdateDeviceList();
   2.112 -        SDL_HIDAPI_discovery.m_bHaveDevicesChanged = SDL_FALSE;
   2.113 +    if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
   2.114 +        HIDAPI_UpdateDiscovery();
   2.115 +        if (SDL_HIDAPI_discovery.m_bHaveDevicesChanged) {
   2.116 +            /* FIXME: We probably need to schedule an update in a few seconds as well */
   2.117 +            HIDAPI_UpdateDeviceList();
   2.118 +            SDL_HIDAPI_discovery.m_bHaveDevicesChanged = SDL_FALSE;
   2.119 +        }
   2.120 +        SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
   2.121      }
   2.122  }
   2.123  
   2.124 @@ -859,18 +867,18 @@
   2.125      SDL_HIDAPI_Device *device;
   2.126  
   2.127      /* Update the devices, which may change connected joysticks and send events */
   2.128 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.129  
   2.130      /* Prepare the existing device list */
   2.131 -    device = SDL_HIDAPI_devices;
   2.132 -    while (device) {
   2.133 -        if (device->driver) {
   2.134 -            device->driver->UpdateDevice(device);
   2.135 +    if (SDL_AtomicTryLock(&SDL_HIDAPI_spinlock)) {
   2.136 +        device = SDL_HIDAPI_devices;
   2.137 +        while (device) {
   2.138 +            if (device->driver) {
   2.139 +                device->driver->UpdateDevice(device);
   2.140 +            }
   2.141 +            device = device->next;
   2.142          }
   2.143 -        device = device->next;
   2.144 +        SDL_AtomicUnlock(&SDL_HIDAPI_spinlock);
   2.145      }
   2.146 -
   2.147 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.148  }
   2.149  
   2.150  static const char *
   2.151 @@ -879,13 +887,11 @@
   2.152      SDL_HIDAPI_Device *device;
   2.153      const char *name = NULL;
   2.154  
   2.155 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.156      device = HIDAPI_GetDeviceByIndex(device_index, NULL);
   2.157      if (device) {
   2.158          /* FIXME: The device could be freed after this name is returned... */
   2.159          name = device->name;
   2.160      }
   2.161 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.162  
   2.163      return name;
   2.164  }
   2.165 @@ -897,12 +903,10 @@
   2.166      SDL_JoystickID instance_id;
   2.167      int player_index = -1;
   2.168  
   2.169 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.170      device = HIDAPI_GetDeviceByIndex(device_index, &instance_id);
   2.171      if (device) {
   2.172          player_index = device->driver->GetDevicePlayerIndex(device, instance_id);
   2.173      }
   2.174 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.175  
   2.176      return player_index;
   2.177  }
   2.178 @@ -913,12 +917,10 @@
   2.179      SDL_HIDAPI_Device *device;
   2.180      SDL_JoystickID instance_id;
   2.181  
   2.182 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.183      device = HIDAPI_GetDeviceByIndex(device_index, &instance_id);
   2.184      if (device) {
   2.185          device->driver->SetDevicePlayerIndex(device, instance_id, player_index);
   2.186      }
   2.187 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.188  }
   2.189  
   2.190  static SDL_JoystickGUID
   2.191 @@ -927,14 +929,12 @@
   2.192      SDL_HIDAPI_Device *device;
   2.193      SDL_JoystickGUID guid;
   2.194  
   2.195 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.196      device = HIDAPI_GetDeviceByIndex(device_index, NULL);
   2.197      if (device) {
   2.198          SDL_memcpy(&guid, &device->guid, sizeof(guid));
   2.199      } else {
   2.200          SDL_zero(guid);
   2.201      }
   2.202 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.203  
   2.204      return guid;
   2.205  }
   2.206 @@ -943,9 +943,7 @@
   2.207  HIDAPI_JoystickGetDeviceInstanceID(int device_index)
   2.208  {
   2.209      SDL_JoystickID joystickID = -1;
   2.210 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.211      HIDAPI_GetDeviceByIndex(device_index, &joystickID);
   2.212 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.213      return joystickID;
   2.214  }
   2.215  
   2.216 @@ -976,7 +974,6 @@
   2.217  {
   2.218      int result;
   2.219  
   2.220 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.221      if (joystick->hwdata) {
   2.222          SDL_HIDAPI_Device *device = joystick->hwdata->device;
   2.223  
   2.224 @@ -985,7 +982,6 @@
   2.225          SDL_SetError("Rumble failed, device disconnected");
   2.226          result = -1;
   2.227      }
   2.228 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.229  
   2.230      return result;
   2.231  }
   2.232 @@ -999,7 +995,6 @@
   2.233  static void
   2.234  HIDAPI_JoystickClose(SDL_Joystick * joystick)
   2.235  {
   2.236 -    SDL_LockMutex(SDL_HIDAPI_mutex);
   2.237      if (joystick->hwdata) {
   2.238          SDL_HIDAPI_Device *device = joystick->hwdata->device;
   2.239  
   2.240 @@ -1008,7 +1003,6 @@
   2.241          SDL_free(joystick->hwdata);
   2.242          joystick->hwdata = NULL;
   2.243      }
   2.244 -    SDL_UnlockMutex(SDL_HIDAPI_mutex);
   2.245  }
   2.246  
   2.247  static void
   2.248 @@ -1029,7 +1023,6 @@
   2.249      }
   2.250      SDL_DelHintCallback(SDL_HINT_JOYSTICK_HIDAPI,
   2.251                          SDL_HIDAPIDriverHintChanged, NULL);
   2.252 -    SDL_DestroyMutex(SDL_HIDAPI_mutex);
   2.253  
   2.254      hid_exit();
   2.255