From 333935ff3f8fa71193076e72da2cbcd6e56adcd9 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 17 Oct 2022 11:10:53 -0700 Subject: [PATCH] Make sure we completely unlock joysticks when opening HIDAPI devices Also lock the joysticks when adding and removing Android joysticks --- src/joystick/SDL_joystick.c | 8 ++++- src/joystick/SDL_joystick_c.h | 3 ++ src/joystick/android/SDL_sysjoystick.c | 38 +++++++++++++++++------- src/joystick/hidapi/SDL_hidapijoystick.c | 14 ++++++--- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/joystick/SDL_joystick.c b/src/joystick/SDL_joystick.c index 2f6d9fb59..cc29a72d8 100644 --- a/src/joystick/SDL_joystick.c +++ b/src/joystick/SDL_joystick.c @@ -168,10 +168,16 @@ SDL_UnlockJoysticks(void) } } +SDL_bool +SDL_JoysticksLocked(void) +{ + return (SDL_joysticks_locked > 0) ? SDL_TRUE : SDL_FALSE; +} + void SDL_AssertJoysticksLocked(void) { - SDL_assert(SDL_joysticks_locked > 0); + SDL_assert(SDL_JoysticksLocked()); } /* diff --git a/src/joystick/SDL_joystick_c.h b/src/joystick/SDL_joystick_c.h index 7ed3e6318..3ded9763e 100644 --- a/src/joystick/SDL_joystick_c.h +++ b/src/joystick/SDL_joystick_c.h @@ -45,6 +45,9 @@ extern SDL_bool SDL_JoysticksInitialized(void); /* Return whether the joystick system is shutting down */ extern SDL_bool SDL_JoysticksQuitting(void); +/* Return whether the joysticks are currently locked */ +extern SDL_bool SDL_JoysticksLocked(void); + /* Make sure we currently have the joysticks locked */ extern void SDL_AssertJoysticksLocked(void); diff --git a/src/joystick/android/SDL_sysjoystick.c b/src/joystick/android/SDL_sysjoystick.c index 558d0d7fd..a1c8b8898 100644 --- a/src/joystick/android/SDL_sysjoystick.c +++ b/src/joystick/android/SDL_sysjoystick.c @@ -317,23 +317,25 @@ Android_AddJoystick(int device_id, const char *name, const char *desc, int vendo SDL_JoystickGUID guid; int i; int axis_mask; + int result = -1; + SDL_LockJoysticks(); if (!SDL_GetHintBoolean(SDL_HINT_TV_REMOTE_AS_JOYSTICK, SDL_TRUE)) { /* Ignore devices that aren't actually controllers (e.g. remotes), they'll be handled as keyboard input */ if (naxes < 2 && nhats < 1) { - return -1; + goto done; } } - + if (JoystickByDeviceId(device_id) != NULL || name == NULL) { - return -1; + goto done; } #ifdef SDL_JOYSTICK_HIDAPI if (HIDAPI_IsDevicePresent(vendor_id, product_id, 0, name)) { /* The HIDAPI driver is taking care of this device */ - return -1; + goto done; } #endif @@ -377,7 +379,7 @@ Android_AddJoystick(int device_id, const char *name, const char *desc, int vendo item = (SDL_joylist_item *) SDL_malloc(sizeof (SDL_joylist_item)); if (item == NULL) { - return -1; + goto done; } SDL_zerop(item); @@ -385,8 +387,8 @@ Android_AddJoystick(int device_id, const char *name, const char *desc, int vendo item->device_id = device_id; item->name = SDL_CreateJoystickName(vendor_id, product_id, NULL, name); if (item->name == NULL) { - SDL_free(item); - return -1; + SDL_free(item); + goto done; } item->is_accelerometer = is_accelerometer; @@ -415,11 +417,16 @@ Android_AddJoystick(int device_id, const char *name, const char *desc, int vendo SDL_PrivateJoystickAdded(item->device_instance); + result = numjoysticks; + #ifdef DEBUG_JOYSTICK SDL_Log("Added joystick %s with device_id %d", item->name, device_id); #endif - return numjoysticks; +done: + SDL_UnlockJoysticks(); + + return result; } int @@ -427,7 +434,10 @@ Android_RemoveJoystick(int device_id) { SDL_joylist_item *item = SDL_joylist; SDL_joylist_item *prev = NULL; + int result = -1; + SDL_LockJoysticks(); + /* Don't call JoystickByDeviceId here or there'll be an infinite loop! */ while (item != NULL) { if (item->device_id == device_id) { @@ -438,7 +448,7 @@ Android_RemoveJoystick(int device_id) } if (item == NULL) { - return -1; + goto done; } if (item->joystick) { @@ -460,13 +470,19 @@ Android_RemoveJoystick(int device_id) SDL_PrivateJoystickRemoved(item->device_instance); + result = numjoysticks; + #ifdef DEBUG_JOYSTICK SDL_Log("Removed joystick with device_id %d", device_id); #endif - + SDL_free(item->name); SDL_free(item); - return numjoysticks; + +done: + SDL_UnlockJoysticks(); + + return result; } diff --git a/src/joystick/hidapi/SDL_hidapijoystick.c b/src/joystick/hidapi/SDL_hidapijoystick.c index 8cf9596bb..37bed785a 100644 --- a/src/joystick/hidapi/SDL_hidapijoystick.c +++ b/src/joystick/hidapi/SDL_hidapijoystick.c @@ -368,15 +368,21 @@ HIDAPI_SetupDeviceDriver(SDL_HIDAPI_Device *device, SDL_bool *removed) * * See https://github.com/libsdl-org/SDL/issues/6347 for details */ + int lock_count = 0; SDL_HIDAPI_Device *curr; SDL_hid_device *dev; - char *path; + char *path = SDL_strdup(device->path); SDL_AssertJoysticksLocked(); - path = SDL_strdup(device->path); - SDL_UnlockJoysticks(); + while (SDL_JoysticksLocked()) { + ++lock_count; + SDL_UnlockJoysticks(); + } dev = SDL_hid_open_path(path, 0); - SDL_LockJoysticks(); + while (lock_count > 0) { + --lock_count; + SDL_LockJoysticks(); + } SDL_free(path); /* Make sure the device didn't get removed while opening the HID path */