From a5250171395eeeb5348be1362afa5f9ab9d7d754 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Thu, 8 Dec 2016 10:13:45 -0800 Subject: [PATCH] Protect the game controller API the same way the joystick API is protected from multi-threaded access --- src/joystick/SDL_gamecontroller.c | 28 +++++++++++++++++++++++----- src/joystick/SDL_joystick.c | 23 +++++++++++++++++------ src/joystick/SDL_joystick_c.h | 3 +++ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/joystick/SDL_gamecontroller.c b/src/joystick/SDL_gamecontroller.c index b304f6ad493a5..e41bae6cc9b9b 100644 --- a/src/joystick/SDL_gamecontroller.c +++ b/src/joystick/SDL_gamecontroller.c @@ -25,6 +25,7 @@ #include "SDL_events.h" #include "SDL_assert.h" #include "SDL_sysjoystick.h" +#include "SDL_joystick_c.h" #include "SDL_hints.h" #include "SDL_gamecontrollerdb.h" @@ -1086,12 +1087,15 @@ SDL_GameControllerOpen(int device_index) return (NULL); } + SDL_LockJoystickList(); + gamecontrollerlist = SDL_gamecontrollers; /* If the controller is already open, return it */ while (gamecontrollerlist) { if (SDL_SYS_GetInstanceIdOfDeviceIndex(device_index) == gamecontrollerlist->joystick->instance_id) { gamecontroller = gamecontrollerlist; ++gamecontroller->ref_count; + SDL_UnlockJoystickList(); return (gamecontroller); } gamecontrollerlist = gamecontrollerlist->next; @@ -1101,13 +1105,15 @@ SDL_GameControllerOpen(int device_index) pSupportedController = SDL_PrivateGetControllerMapping(device_index); if (!pSupportedController) { SDL_SetError("Couldn't find mapping for device (%d)", device_index); - return (NULL); + SDL_UnlockJoystickList(); + return NULL; } /* Create and initialize the joystick */ gamecontroller = (SDL_GameController *) SDL_malloc((sizeof *gamecontroller)); if (gamecontroller == NULL) { SDL_OutOfMemory(); + SDL_UnlockJoystickList(); return NULL; } @@ -1115,6 +1121,7 @@ SDL_GameControllerOpen(int device_index) gamecontroller->joystick = SDL_JoystickOpen(device_index); if (!gamecontroller->joystick) { SDL_free(gamecontroller); + SDL_UnlockJoystickList(); return NULL; } @@ -1140,7 +1147,7 @@ SDL_GameControllerOpen(int device_index) gamecontroller->next = SDL_gamecontrollers; SDL_gamecontrollers = gamecontroller; - SDL_SYS_JoystickUpdate(gamecontroller->joystick); + SDL_UnlockJoystickList(); return (gamecontroller); } @@ -1274,14 +1281,18 @@ SDL_Joystick *SDL_GameControllerGetJoystick(SDL_GameController * gamecontroller) SDL_GameController * SDL_GameControllerFromInstanceID(SDL_JoystickID joyid) { - SDL_GameController *gamecontroller = SDL_gamecontrollers; + SDL_GameController *gamecontroller; + + SDL_LockJoystickList(); + gamecontroller = SDL_gamecontrollers; while (gamecontroller) { if (gamecontroller->joystick->instance_id == joyid) { + SDL_UnlockJoystickList(); return gamecontroller; } gamecontroller = gamecontroller->next; } - + SDL_UnlockJoystickList(); return NULL; } @@ -1344,8 +1355,11 @@ SDL_GameControllerClose(SDL_GameController * gamecontroller) if (!gamecontroller) return; + SDL_LockJoystickList(); + /* First decrement ref count */ if (--gamecontroller->ref_count > 0) { + SDL_UnlockJoystickList(); return; } @@ -1361,7 +1375,6 @@ SDL_GameControllerClose(SDL_GameController * gamecontroller) } else { SDL_gamecontrollers = gamecontroller->next; } - break; } gamecontrollerlistprev = gamecontrollerlist; @@ -1369,6 +1382,8 @@ SDL_GameControllerClose(SDL_GameController * gamecontroller) } SDL_free(gamecontroller); + + SDL_UnlockJoystickList(); } @@ -1379,10 +1394,13 @@ void SDL_GameControllerQuit(void) { ControllerMapping_t *pControllerMap; + + SDL_LockJoystickList(); while (SDL_gamecontrollers) { SDL_gamecontrollers->ref_count = 1; SDL_GameControllerClose(SDL_gamecontrollers); } + SDL_UnlockJoystickList(); while (s_pSupportedControllers) { pControllerMap = s_pSupportedControllers; diff --git a/src/joystick/SDL_joystick.c b/src/joystick/SDL_joystick.c index b60dbfc0b5fb8..05e8175f744af 100644 --- a/src/joystick/SDL_joystick.c +++ b/src/joystick/SDL_joystick.c @@ -37,16 +37,16 @@ static SDL_Joystick *SDL_joysticks = NULL; static SDL_Joystick *SDL_updating_joystick = NULL; static SDL_mutex *SDL_joystick_lock = NULL; /* This needs to support recursive locks */ -static void -SDL_LockJoystickList() +void +SDL_LockJoystickList(void) { if (SDL_joystick_lock) { SDL_LockMutex(SDL_joystick_lock); } } -static void -SDL_UnlockJoystickList() +void +SDL_UnlockJoystickList(void) { if (SDL_joystick_lock) { SDL_UnlockMutex(SDL_joystick_lock); @@ -216,10 +216,10 @@ SDL_JoystickOpen(int device_index) joystick->next = SDL_joysticks; SDL_joysticks = joystick; - SDL_SYS_JoystickUpdate(joystick); - SDL_UnlockJoystickList(); + SDL_SYS_JoystickUpdate(joystick); + return (joystick); } @@ -787,6 +787,12 @@ SDL_JoystickUpdate(void) SDL_LockJoystickList(); + if (SDL_updating_joystick) { + /* The joysticks are already being updated */ + SDL_UnlockJoystickList(); + return; + } + for (joystick = SDL_joysticks; joystick; joystick = joysticknext) { /* save off the next pointer, the Update call may cause a joystick removed event * and cause our joystick pointer to be freed @@ -795,6 +801,9 @@ SDL_JoystickUpdate(void) SDL_updating_joystick = joystick; + /* Make sure the list is unlocked while dispatching events to prevent application deadlocks */ + SDL_UnlockJoystickList(); + SDL_SYS_JoystickUpdate(joystick); if (joystick->force_recentering) { @@ -816,6 +825,8 @@ SDL_JoystickUpdate(void) joystick->force_recentering = SDL_FALSE; } + SDL_LockJoystickList(); + SDL_updating_joystick = NULL; /* If the joystick was closed while updating, free it here */ diff --git a/src/joystick/SDL_joystick_c.h b/src/joystick/SDL_joystick_c.h index cb9c92544cb33..21b5b1674634b 100644 --- a/src/joystick/SDL_joystick_c.h +++ b/src/joystick/SDL_joystick_c.h @@ -31,6 +31,9 @@ extern void SDL_JoystickQuit(void); extern int SDL_GameControllerInit(void); extern void SDL_GameControllerQuit(void); +/* Locking for multi-threaded access to the joystick API */ +extern void SDL_LockJoystickList(void); +extern void SDL_UnlockJoystickList(void); /* Internal event queueing functions */ extern void SDL_PrivateJoystickAdded(int device_index);