From c9723c407ff0792b5bb5708754fa760ae9144da6 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 7 Dec 2020 09:38:21 -0800 Subject: [PATCH] Fixed potential hang in joystick close if the rumble thread is blocked 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. --- src/joystick/hidapi/SDL_hidapi_rumble.c | 17 ++++++++++++++--- src/joystick/hidapi/SDL_hidapijoystick.c | 22 +++++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/joystick/hidapi/SDL_hidapi_rumble.c b/src/joystick/hidapi/SDL_hidapi_rumble.c index 9aae15eee..a9477d985 100644 --- 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 index 5684e55ca..3dc942fd9 100644 --- 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; }