Commit a5250171395eeeb5348be1362afa5f9ab9d7d754

Sam Lantinga 2016-12-08T10:13:45

Protect the game controller API the same way the joystick API is protected from multi-threaded access

diff --git a/src/joystick/SDL_gamecontroller.c b/src/joystick/SDL_gamecontroller.c
index b304f6a..e41bae6 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 b60dbfc..05e8175 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 cb9c925..21b5b16 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);