We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5195 - Replugging in “mixed” controller types crashes on macOS in SDL 2.0.13
Summary: Replugging in “mixed” controller types crashes on macOS in SDL 2.0.13
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: joystick (show other bugs)
Version: 2.0.13
Hardware: x86 Mac OS X (All)
: P2 critical
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-16 01:30 UTC by RustyM
Modified: 2021-01-14 23:03 UTC (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description RustyM 2020-06-16 01:30:58 UTC 
This is related to Bug 5034, but crashes under a somewhat different condition.

In the latest tip (changeset 13914) or with the SDL 2.0.12 source + David’s 5034 patch, unplugging and then replugging in certain controller types on macOS will crash. A mix of new controllers like Switch Pro, PS4 and Xbox One all work without issue. But if a controller without a rumble function, like many SNES retro USB gamepads, is mixed with a PS4 or Switch Pro controller it will crash.

File: joystick/darwin/SDL_sysjoystick.c
Function: static recDevice *FreeDevice(recDevice *removeDevice)
On line 159: while (device->pNext != removeDevice) {
Causes: Thread 1: EXC_BAD_ACCESS (code=1, address=0x188)

This can be reproduced in testgamecontroller" by starting the test program with both a “retro” controller plugged in and a “modern rumble” controller (Switch Pro/PS4). This may crash on launch, but it depends on which controller ends up as device 0. If it doesn’t crash, unplug the “modern rumble” controller and plug it back in.

Some of the "retro" controllers I’ve seen this crash with:
- iBuffalo SNES Controller
- 8Bitdo SN30 Gamepad (in MacOS mode)
- Retrolink NES Controller
- HuiJia SNES Controller Adaptor

The issue appears macOS specific. Seen on 10.12.6 and 10.14.6. Not seen on Windows 10.


Stack trace from latest tip (changeset 13914):

thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x188)
frame #0: 0x0000000100188ae5 testgamecontroller`FreeDevice(removeDevice=0x0000000100d1be30) at SDL_sysjoystick.c:159
frame #1: 0x00000001001881b4 testgamecontroller`JoystickDeviceWasAddedCallback(ctx=0x0000000000000000, res=0, sender=0x0000000100f00890, ioHIDDeviceObject=0x0000000100d101a0) at SDL_sysjoystick.c:561
frame #2: 0x00007fff99c20188 IOKit`__IOHIDManagerDeviceApplier + 156
frame #3: 0x00007fff99c2127b IOKit`__IOHIDManagerDeviceAdded + 339
frame #4: 0x00007fff99bec52e IOKit`IODispatchCalloutFromCFMessage + 177
frame #5: 0x00007fff97c792f3 CoreFoundation`__CFMachPortPerform + 291
frame #6: 0x00007fff97c791b9 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 41
frame #7: 0x00007fff97c79131 CoreFoundation`__CFRunLoopDoSource1 + 465
frame #8: 0x00007fff97c71035 CoreFoundation`__CFRunLoopRun + 2405
frame #9: 0x00007fff97c70474 CoreFoundation`CFRunLoopRunSpecific + 420
frame #10: 0x0000000100187621 testgamecontroller`DARWIN_JoystickDetect at SDL_sysjoystick.c:731
frame #11: 0x000000010018f5f8 testgamecontroller`SDL_JoystickUpdate_REAL at SDL_joystick.c:1410
frame #12: 0x0000000100162128 testgamecontroller`SDL_PumpEvents_REAL at SDL_events.c:692
frame #13: 0x00000001001621a5 testgamecontroller`SDL_WaitEventTimeout_REAL(event=0x00007fff5fbff2f0, timeout=0) at SDL_events.c:729
frame #14: 0x0000000100162167 testgamecontroller`SDL_PollEvent_REAL(event=0x00007fff5fbff2f0) at SDL_events.c:711
frame #15: 0x0000000100118684 testgamecontroller`SDL_PollEvent(a=0x00007fff5fbff2f0) at SDL_dynapi_procs.h:153
frame #16: 0x0000000100001bbd testgamecontroller`loop(arg=0x0000000000000000) at testgamecontroller.c:128
frame #17: 0x00000001000026d5 testgamecontroller`main(argc=2, argv=0x00007fff5fbff480) at testgamecontroller.c:331
frame #18: 0x0000000100001b24 testgamecontroller`start + 52
Comment 1 RustyM 2020-06-17 15:58:24 UTC 
The while loop in FreeDevice() assumes that every device is not NULL.

    recDevice *device = gpDeviceList;
    while (device->pNext != removeDevice) {
        device = device->pNext;
    }
    device->pNext = pDeviceNext;

So maybe we should check for NULL here? Or instead prevent adding NULL devices to the list in the first place? Checking device for NULL before entering the loop appears to work.

    recDevice *device = gpDeviceList;
    if (!device) {
        while (device->pNext != removeDevice) {
            device = device->pNext;
        }
    }
    device->pNext = pDeviceNext;

I don’t really understand how SDL is tracking devices, so I suspect this is a naive solution. This seems to fix the crash, but I fear we are no longer freeing a device or doing something else horrible.

Also I apologize if this bug shouldn’t be marked “critical”. I chose it because Bug 5034 was marked the same way.
Comment 2 Mathias Kaerlev 2021-01-14 13:16:52 UTC 
Looks like we have reproduced this same issue on 2.0.14 on Mac with a PS5 and Nintendo Switch Pro Controller.

This might be caused by the early FreeDevice calls in JoystickDeviceWasAddedCallback before the device is added to the global device list gpDeviceList.
This will likely cause FreeDevice to crash, since it will never find the device in the global device list:

            while (device->pNext != removeDevice) {
                device = device->pNext;
            }
Comment 3 Sam Lantinga 2021-01-14 23:03:42 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/b1d2cb0484aa