From fc720321b32ff59ed9852a2cc5f133f3959b2b97 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Thu, 6 Oct 2022 18:23:07 -0700 Subject: [PATCH] Fix rare deadlock when opening a HID controller on Android Fixes https://github.com/libsdl-org/SDL/issues/6347 --- src/joystick/hidapi/SDL_hidapijoystick.c | 60 ++++++++++++++++++++---- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/src/joystick/hidapi/SDL_hidapijoystick.c b/src/joystick/hidapi/SDL_hidapijoystick.c index 25db216e3..ee56e5669 100644 --- a/src/joystick/hidapi/SDL_hidapijoystick.c +++ b/src/joystick/hidapi/SDL_hidapijoystick.c @@ -330,8 +330,10 @@ HIDAPI_CleanupDeviceDriver(SDL_HIDAPI_Device *device) } static void -HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device) +HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device, SDL_bool *removed) { + *removed = SDL_FALSE; + if (device->driver) { SDL_bool enabled; @@ -359,14 +361,44 @@ HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device) /* Make sure we can open the device and leave it open for the driver */ if (device->num_children == 0) { - device->dev = SDL_hid_open_path(device->path, 0); - if (!device->dev) { + /* On Android we need to leave joysticks unlocked because it calls + * out to the main thread for permissions and the main thread can + * be in the process of handling controller input. + * + * See https://github.com/libsdl-org/SDL/issues/6347 for details + */ + SDL_HIDAPI_Device *curr; + SDL_hid_device *dev; + char *path; + + SDL_AssertJoysticksLocked(); + path = SDL_strdup(device->path); + SDL_UnlockJoysticks(); + dev = SDL_hid_open_path(path, 0); + SDL_LockJoysticks(); + SDL_free(path); + + /* Make sure the device didn't get removed while opening the HID path */ + for (curr = SDL_HIDAPI_devices; curr && curr != device; curr = curr->next) { + continue; + } + if (!curr) { + *removed = SDL_TRUE; + if (dev) { + SDL_hid_close(dev); + } + return; + } + + if (!dev) { SDL_LogDebug(SDL_LOG_CATEGORY_INPUT, "HIDAPI_SetupDeviceDriver() couldn't open %s: %s\n", device->path, SDL_GetError()); return; } - SDL_hid_set_nonblocking(device->dev, 1); + SDL_hid_set_nonblocking(dev, 1); + + device->dev = dev; } device->driver = HIDAPI_GetDeviceDriver(device); @@ -388,6 +420,7 @@ SDL_HIDAPI_UpdateDrivers(void) { int i; SDL_HIDAPI_Device *device; + SDL_bool removed; SDL_HIDAPI_numdrivers = 0; for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) { @@ -398,9 +431,15 @@ SDL_HIDAPI_UpdateDrivers(void) } } - for (device = SDL_HIDAPI_devices; device; device = device->next) { - HIDAPI_SetupDeviceDriver(device); - } + removed = SDL_FALSE; + do { + for (device = SDL_HIDAPI_devices; device; device = device->next) { + HIDAPI_SetupDeviceDriver(device, &removed); + if (removed) { + break; + } + } + } while (removed); } static void SDLCALL @@ -685,6 +724,7 @@ HIDAPI_AddDevice(const struct SDL_hid_device_info *info, int num_children, SDL_H { SDL_HIDAPI_Device *device; SDL_HIDAPI_Device *curr, *last = NULL; + SDL_bool removed; for (curr = SDL_HIDAPI_devices, last = NULL; curr; last = curr, curr = curr->next) { continue; @@ -762,7 +802,11 @@ HIDAPI_AddDevice(const struct SDL_hid_device_info *info, int num_children, SDL_H SDL_HIDAPI_devices = device; } - HIDAPI_SetupDeviceDriver(device); + removed = SDL_FALSE; + HIDAPI_SetupDeviceDriver(device, &removed); + if (removed) { + return NULL; + } #ifdef DEBUG_HIDAPI SDL_Log("Added 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");