WiimoteReal: Fix Wiimote disconnection causing Dolphin to crash on macOS #10014
+139
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ported from libusb's hidapi fork. Original patch by Youw (cdc473dfe43f6432dda7ad53d7656b8ae8ff968b).
leoetlino
approved these changes
Sep 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
When disconnecting a Wiimote on macOS (whether by the Bluetooth menu, by pulling out the batteries, etc...), Dolphin may crash in hidapi code.
The first commit changes the device removal callback to use
CFRunLoopGetCurrent()instead of getting the run loop ref from the context. For whatever reason (macOS bug?), while the callback is deregistered for closedhid_devices inhid_close(), IOKit may occasionally call the callback withhid_devicepointers that have already been closed andfree'd. Because the device removal callback is always ran on the device's run loop, it is safer to useCFRunLoopGetCurrent()instead of accessingcontext->run_loop.The second commit ports a patch from libusb's fork of hidapi. On really old versions of macOS, calling
IOHIDDeviceClose()on a handle to a device that has been physically removed from the system may cause a crash. The workaround was to not callIOHIDDeviceClose()on disconnected devices. While this bug has apparently been fixed on some version since then, the behaviour has since changed. Now, there may be a crash on macOS Catalina and higher ifIOHIDDeviceClose()is not called. The patch makeshid_close()always callIOHIDDeviceClose()on modern versions of macOS.The third commit fixes a minor bug.
WiimoteReal::IOReadignores all read results, including errors, if the real Wiimote isn't linked. This causes lots of log spam if a Wiimote is disconnected while no game is running. The fix is to move the error check before the link check.I've connected and disconnected a Wiimote 20+ times on my Mac, and Dolphin has not crashed once with this PR.
These hidapi patches have not been submitted upstream, as signall11 has apparently abandoned hidapi. I have tried updating the code in
Externalsto libusb's fork, but Wiimote connection never worked for me. I suspect it is related to the ongoing issue with how paths are generated on newer versions (hidapi now uses IORegistry path strings, which are the same between all Bluetooth HID devices).Because of this, I think it is better to just patch the version in Externals as it still works fine on current macOS.