Fix for Bug 5034 - Replugging in a controller crashes on macOS in SDL 2.0.12
authorDavid Ludwig <dludwig@pobox.com>
Tue, 17 Mar 2020 17:34:24 -0400
changeset 13646784ce9766fb9
parent 13645 f7fc52b64177
child 13647 c8f27a5e868e
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
src/joystick/darwin/SDL_sysjoystick_c.h
     1.1 --- a/src/joystick/darwin/SDL_sysjoystick.c	Tue Mar 17 14:18:05 2020 -0700
     1.2 +++ b/src/joystick/darwin/SDL_sysjoystick.c	Tue Mar 17 17:34:24 2020 -0400
     1.3 @@ -127,11 +127,28 @@
     1.4      recDevice *pDeviceNext = NULL;
     1.5      if (removeDevice) {
     1.6          if (removeDevice->deviceRef) {
     1.7 -            IOHIDDeviceUnscheduleFromRunLoop(removeDevice->deviceRef, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE);
     1.8 +            if (removeDevice->runLoopAttached) {
     1.9 +                /* Calling IOHIDDeviceUnscheduleFromRunLoop without a prior,
    1.10 +                 * paired call to IOHIDDeviceScheduleWithRunLoop can lead
    1.11 +                 * to crashes in MacOS 10.14.x and earlier.  This doesn't
    1.12 +                 * appear to be a problem in MacOS 10.15.x, but we'll
    1.13 +                 * do it anyways.  (Part-of fix for Bug 5034)
    1.14 +                 */
    1.15 +                IOHIDDeviceUnscheduleFromRunLoop(removeDevice->deviceRef, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE);
    1.16 +            }
    1.17              CFRelease(removeDevice->deviceRef);
    1.18              removeDevice->deviceRef = NULL;
    1.19          }
    1.20  
    1.21 +        /* clear out any reference to removeDevice from an associated,
    1.22 +         * live instance of SDL_Joystick  (Part-of fix for Bug 5034)
    1.23 +         */
    1.24 +        SDL_LockJoysticks();
    1.25 +        if (removeDevice->joystick) {
    1.26 +            removeDevice->joystick->hwdata = NULL;
    1.27 +        }
    1.28 +        SDL_UnlockJoysticks();
    1.29 +
    1.30          /* save next device prior to disposing of this device */
    1.31          pDeviceNext = removeDevice->pNext;
    1.32  
    1.33 @@ -398,6 +415,7 @@
    1.34      }
    1.35  }
    1.36  
    1.37 +
    1.38  static SDL_bool
    1.39  GetDeviceInfo(IOHIDDeviceRef hidDevice, recDevice *pDevice)
    1.40  {
    1.41 @@ -552,6 +570,7 @@
    1.42      /* Get notified when this device is disconnected. */
    1.43      IOHIDDeviceRegisterRemovalCallback(ioHIDDeviceObject, JoystickDeviceWasRemovedCallback, device);
    1.44      IOHIDDeviceScheduleWithRunLoop(ioHIDDeviceObject, CFRunLoopGetCurrent(), SDL_JOYSTICK_RUNLOOP_MODE);
    1.45 +    device->runLoopAttached = SDL_TRUE;
    1.46  
    1.47      /* Allocate an instance ID for this device */
    1.48      device->instance_id = SDL_GetNextJoystickInstanceID();
    1.49 @@ -760,6 +779,7 @@
    1.50  
    1.51      joystick->instance_id = device->instance_id;
    1.52      joystick->hwdata = device;
    1.53 +    device->joystick = joystick;
    1.54      joystick->name = device->product;
    1.55  
    1.56      joystick->naxes = device->axes;
    1.57 @@ -870,6 +890,10 @@
    1.58  
    1.59      /* Scale and average the two rumble strengths */
    1.60      Sint16 magnitude = (Sint16)(((low_frequency_rumble / 2) + (high_frequency_rumble / 2)) / 2);
    1.61 +    
    1.62 +    if (!device) {
    1.63 +        return SDL_SetError("Rumble failed, device disconnected");
    1.64 +    }
    1.65  
    1.66      if (!device->ffservice) {
    1.67          return SDL_Unsupported();
    1.68 @@ -1007,6 +1031,10 @@
    1.69  static void
    1.70  DARWIN_JoystickClose(SDL_Joystick * joystick)
    1.71  {
    1.72 +    recDevice *device = joystick->hwdata;
    1.73 +    if (device) {
    1.74 +        device->joystick = NULL;
    1.75 +    }
    1.76  }
    1.77  
    1.78  static void
     2.1 --- a/src/joystick/darwin/SDL_sysjoystick_c.h	Tue Mar 17 14:18:05 2020 -0700
     2.2 +++ b/src/joystick/darwin/SDL_sysjoystick_c.h	Tue Mar 17 17:34:24 2020 -0400
     2.3 @@ -66,6 +66,8 @@
     2.4      recElement *firstHat;
     2.5  
     2.6      SDL_bool removed;
     2.7 +    SDL_Joystick *joystick;
     2.8 +    SDL_bool runLoopAttached;   /* is 'deviceRef' attached to a CFRunLoop? */
     2.9  
    2.10      int instance_id;
    2.11      SDL_JoystickGUID guid;