From 7e5340c5ac1a07ff76b4a532e8c534d9c849a4fb Mon Sep 17 00:00:00 2001 From: David Ludwig Date: Tue, 17 Mar 2020 17:34:24 -0400 Subject: [PATCH] Fix for Bug 5034 - Replugging in a controller crashes on macOS in SDL 2.0.12 This is a multi-part fix, and is the 2nd attempt at a fix for Bug 5034. Here are the problems being addressed: 1. On macOS 10.14.x and earlier, trying to call IOHIDDeviceUnscheduleFromRunLoop without a prior, paired call to IOHIDDeviceScheduleWithRunLoop, appears to lead to a crash. A per-device flag has been added to make sure that these calls are paired. 2. DARWIN_JoystickDetect was free'ing its SDL_joystick's hwdata field (via FreeDevice) without setting it to NULL, and DARWIN_JoystickRumble wasn't checking for a NULL hwdata. FreeDevice will now set hwdata to NULL and DARWIN_JoystickRumble will check for a NULL hwdata. --- src/joystick/darwin/SDL_sysjoystick.c | 30 ++++++++++++++++++++++++- src/joystick/darwin/SDL_sysjoystick_c.h | 2 ++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/joystick/darwin/SDL_sysjoystick.c b/src/joystick/darwin/SDL_sysjoystick.c index 44d41365fbc51..86debc3f1958a 100644 --- a/src/joystick/darwin/SDL_sysjoystick.c +++ b/src/joystick/darwin/SDL_sysjoystick.c @@ -127,11 +127,28 @@ FreeDevice(recDevice *removeDevice) recDevice *pDeviceNext = NULL; if (removeDevice) { if (removeDevice->deviceRef) { - IOHIDDeviceUnscheduleFromRunLoop(removeDevice->deviceRef, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE); + if (removeDevice->runLoopAttached) { + /* Calling IOHIDDeviceUnscheduleFromRunLoop without a prior, + * paired call to IOHIDDeviceScheduleWithRunLoop can lead + * to crashes in MacOS 10.14.x and earlier. This doesn't + * appear to be a problem in MacOS 10.15.x, but we'll + * do it anyways. (Part-of fix for Bug 5034) + */ + IOHIDDeviceUnscheduleFromRunLoop(removeDevice->deviceRef, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE); + } CFRelease(removeDevice->deviceRef); removeDevice->deviceRef = NULL; } + /* clear out any reference to removeDevice from an associated, + * live instance of SDL_Joystick (Part-of fix for Bug 5034) + */ + SDL_LockJoysticks(); + if (removeDevice->joystick) { + removeDevice->joystick->hwdata = NULL; + } + SDL_UnlockJoysticks(); + /* save next device prior to disposing of this device */ pDeviceNext = removeDevice->pNext; @@ -398,6 +415,7 @@ AddHIDElement(const void *value, void *parameter) } } + static SDL_bool GetDeviceInfo(IOHIDDeviceRef hidDevice, recDevice *pDevice) { @@ -552,6 +570,7 @@ JoystickDeviceWasAddedCallback(void *ctx, IOReturn res, void *sender, IOHIDDevic /* Get notified when this device is disconnected. */ IOHIDDeviceRegisterRemovalCallback(ioHIDDeviceObject, JoystickDeviceWasRemovedCallback, device); IOHIDDeviceScheduleWithRunLoop(ioHIDDeviceObject, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE); + device->runLoopAttached = SDL_TRUE; /* Allocate an instance ID for this device */ device->instance_id = SDL_GetNextJoystickInstanceID(); @@ -760,6 +779,7 @@ DARWIN_JoystickOpen(SDL_Joystick * joystick, int device_index) joystick->instance_id = device->instance_id; joystick->hwdata = device; + device->joystick = joystick; joystick->name = device->product; joystick->naxes = device->axes; @@ -870,6 +890,10 @@ DARWIN_JoystickRumble(SDL_Joystick * joystick, Uint16 low_frequency_rumble, Uint /* Scale and average the two rumble strengths */ Sint16 magnitude = (Sint16)(((low_frequency_rumble / 2) + (high_frequency_rumble / 2)) / 2); + + if (!device) { + return SDL_SetError("Rumble failed, device disconnected"); + } if (!device->ffservice) { return SDL_Unsupported(); @@ -1007,6 +1031,10 @@ DARWIN_JoystickUpdate(SDL_Joystick * joystick) static void DARWIN_JoystickClose(SDL_Joystick * joystick) { + recDevice *device = joystick->hwdata; + if (device) { + device->joystick = NULL; + } } static void diff --git a/src/joystick/darwin/SDL_sysjoystick_c.h b/src/joystick/darwin/SDL_sysjoystick_c.h index 45a5793cfd7e7..4505eccabcca8 100644 --- a/src/joystick/darwin/SDL_sysjoystick_c.h +++ b/src/joystick/darwin/SDL_sysjoystick_c.h @@ -66,6 +66,8 @@ struct joystick_hwdata recElement *firstHat; SDL_bool removed; + SDL_Joystick *joystick; + SDL_bool runLoopAttached; /* is 'deviceRef' attached to a CFRunLoop? */ int instance_id; SDL_JoystickGUID guid;