Fixed potential hang in joystick close if the rumble thread is blocke…

…d for some reason It's still possible to hang when shutting down, if the rumble thread is still hung, but it won't block indefinitely at runtime.
libsdl-org · Dec 7, 2020 · c9723c4 · c9723c4
1 parent 3c68051
commit c9723c4
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 10 deletions.
diff --git a/src/joystick/hidapi/SDL_hidapi_rumble.c b/src/joystick/hidapi/SDL_hidapi_rumble.c
@@ -90,6 +90,8 @@ static int SDL_HIDAPI_RumbleThread(void *data)
 static void
 SDL_HIDAPI_StopRumbleThread(SDL_HIDAPI_RumbleContext *ctx)
 {
+    SDL_HIDAPI_RumbleRequest *request;
+
     SDL_AtomicSet(&ctx->running, SDL_FALSE);
 
     if (ctx->thread) {
@@ -100,9 +102,18 @@ SDL_HIDAPI_StopRumbleThread(SDL_HIDAPI_RumbleContext *ctx)
         ctx->thread = NULL;
     }
 
-    /* This should always be called with an empty queue */
-    SDL_assert(!ctx->requests_head);
-    SDL_assert(!ctx->requests_tail);
+    SDL_LockMutex(ctx->lock);
+    while (ctx->requests_tail) {
+        request = ctx->requests_tail;
+        if (request == ctx->requests_head) {
+            ctx->requests_head = NULL;
+        }
+        ctx->requests_tail = request->prev;
+
+        (void)SDL_AtomicDecRef(&request->device->rumble_pending);
+        SDL_free(request);
+    }
+    SDL_UnlockMutex(ctx->lock);
 
     if (ctx->request_sem) {
         SDL_DestroySemaphore(ctx->request_sem);

diff --git a/src/joystick/hidapi/SDL_hidapijoystick.c b/src/joystick/hidapi/SDL_hidapijoystick.c
@@ -833,6 +833,11 @@ HIDAPI_DelDevice(SDL_HIDAPI_Device *device)
 
             HIDAPI_CleanupDeviceDriver(device);
 
+            /* Make sure the rumble thread is done with this device */
+            while (SDL_AtomicGet(&device->rumble_pending) > 0) {
+                SDL_Delay(10);
+            }
+
             SDL_DestroyMutex(device->dev_lock);
             SDL_free(device->serial);
             SDL_free(device->name);
@@ -1193,10 +1198,13 @@ HIDAPI_JoystickClose(SDL_Joystick * joystick)
 {
     if (joystick->hwdata) {
         SDL_HIDAPI_Device *device = joystick->hwdata->device;
+        int i;
 
-        /* Wait for pending rumble to complete */
-        while (SDL_AtomicGet(&device->rumble_pending) > 0) {
-            SDL_Delay(10);
+        /* Wait up to 30 ms for pending rumble to complete */
+        for (i = 0; i < 3; ++i) {
+            while (SDL_AtomicGet(&device->rumble_pending) > 0) {
+                SDL_Delay(10);
+            }
         }
 
         device->driver->CloseJoystick(device, joystick);
@@ -1215,11 +1223,14 @@ HIDAPI_JoystickQuit(void)
 
     HIDAPI_ShutdownDiscovery();
 
+    SDL_HIDAPI_QuitRumble();
+
     while (SDL_HIDAPI_devices) {
         HIDAPI_DelDevice(SDL_HIDAPI_devices);
     }
 
-    SDL_HIDAPI_QuitRumble();
+    /* Make sure the drivers cleaned up properly */
+    SDL_assert(SDL_HIDAPI_numjoysticks == 0);
 
     for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
         SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
@@ -1230,9 +1241,6 @@ HIDAPI_JoystickQuit(void)
 
     hid_exit();
 
-    /* Make sure the drivers cleaned up properly */
-    SDL_assert(SDL_HIDAPI_numjoysticks == 0);
-
     shutting_down = SDL_FALSE;
     initialized = SDL_FALSE;
 }