Merge pull request #10014 from OatmealDome/wiimote-disconnect-crash

WiimoteReal: Fix Wiimote disconnection causing Dolphin to crash on macOS
This commit is contained in:
Léo Lam 2021-09-13 15:26:02 +02:00 committed by GitHub
commit ee863e6722
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 139 additions and 14 deletions

View File

@ -0,0 +1,42 @@
From b24599cf5bf07f0f4d12e197e61c8f107a042dd4 Mon Sep 17 00:00:00 2001
From: OatmealDome <julian@oatmealdome.me>
Date: Mon, 9 Aug 2021 21:15:17 -0400
Subject: [PATCH] hidapi: Stop current run loop in removal callback instead of
fetching from context
---
Externals/hidapi/mac/hid.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/Externals/hidapi/mac/hid.c b/Externals/hidapi/mac/hid.c
index 70b615d40d..d8055f372c 100644
--- a/Externals/hidapi/mac/hid.c
+++ b/Externals/hidapi/mac/hid.c
@@ -562,11 +562,10 @@ hid_device * HID_API_EXPORT hid_open(unsigned short vendor_id, unsigned short pr
static void hid_device_removal_callback(void *context, IOReturn result,
void *sender)
{
- /* Stop the Run Loop for this device. */
- hid_device *d = context;
-
- d->disconnected = 1;
- CFRunLoopStop(d->run_loop);
+ /* Stop the Run Loop for this device. This callback will always
+ be called on the device's registered run loop, so we can just
+ get the current loop. */
+ CFRunLoopStop(CFRunLoopGetCurrent());
}
/* The Run Loop calls this function for each input report received.
@@ -658,7 +657,7 @@ static void *read_thread(void *param)
while (!dev->shutdown_thread && !dev->disconnected) {
code = CFRunLoopRunInMode(dev->run_loop_mode, 1000/*sec*/, FALSE);
/* Return if the device has been disconnected */
- if (code == kCFRunLoopRunFinished) {
+ if (code == kCFRunLoopRunFinished || code == kCFRunLoopRunStopped) {
dev->disconnected = 1;
break;
}
--
2.30.1 (Apple Git-130)

View File

@ -0,0 +1,70 @@
From 25c85d827aed098d2536f09a68c23780346cd4e1 Mon Sep 17 00:00:00 2001
From: OatmealDome <julian@oatmealdome.me>
Date: Mon, 9 Aug 2021 21:24:10 -0400
Subject: [PATCH] hidapi: Don't leak device handle in macOS 10.10 or newer
Ported from libusb's hidapi fork. Original patch by Youw
(cdc473dfe43f6432dda7ad53d7656b8ae8ff968b).
---
Externals/hidapi/mac/hid.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/Externals/hidapi/mac/hid.c b/Externals/hidapi/mac/hid.c
index d8055f372c..ea945d4dec 100644
--- a/Externals/hidapi/mac/hid.c
+++ b/Externals/hidapi/mac/hid.c
@@ -33,6 +33,9 @@
#include "hidapi.h"
+/* As defined in AppKit.h, but we don't need the entire AppKit for a single constant. */
+extern const double NSAppKitVersionNumber;
+
/* Barrier implementation because Mac OSX doesn't have pthread_barrier.
It also doesn't have clock_gettime(). So much for POSIX and SUSv2.
This implementation came from Brent Priddy and was posted on
@@ -177,6 +180,7 @@ static void free_hid_device(hid_device *dev)
}
static IOHIDManagerRef hid_mgr = 0x0;
+static int is_macos_10_10_or_greater = 0;
#if 0
@@ -390,6 +394,7 @@ static int init_hid_manager(void)
int HID_API_EXPORT hid_init(void)
{
if (!hid_mgr) {
+ is_macos_10_10_or_greater = (NSAppKitVersionNumber >= 1343); /* NSAppKitVersionNumber10_10 */
return init_hid_manager();
}
@@ -989,7 +994,7 @@ void HID_API_EXPORT hid_close(hid_device *dev)
return;
/* Disconnect the report callback before close. */
- if (!dev->disconnected) {
+ if (is_macos_10_10_or_greater || !dev->disconnected) {
IOHIDDeviceRegisterInputReportCallback(
dev->device_handle, dev->input_report_buf, dev->max_input_report_len,
NULL, dev);
@@ -1013,8 +1018,14 @@ void HID_API_EXPORT hid_close(hid_device *dev)
/* Close the OS handle to the device, but only if it's not
been unplugged. If it's been unplugged, then calling
- IOHIDDeviceClose() will crash. */
- if (!dev->disconnected) {
+ IOHIDDeviceClose() will crash.
+
+ UPD: The crash part was true in/until some version of macOS.
+ Starting with macOS 10.15, there is an opposite effect in some environments:
+ crash happenes if IOHIDDeviceClose() is not called.
+ Not leaking a resource in all tested environments.
+ */
+ if (is_macos_10_10_or_greater || !dev->disconnected) {
IOHIDDeviceClose(dev->device_handle, kIOHIDOptionsTypeSeizeDevice);
}
--
2.30.1 (Apple Git-130)

View File

@ -33,6 +33,9 @@
#include "hidapi.h"
/* As defined in AppKit.h, but we don't need the entire AppKit for a single constant. */
extern const double NSAppKitVersionNumber;
/* Barrier implementation because Mac OSX doesn't have pthread_barrier.
It also doesn't have clock_gettime(). So much for POSIX and SUSv2.
This implementation came from Brent Priddy and was posted on
@ -177,6 +180,7 @@ static void free_hid_device(hid_device *dev)
}
static IOHIDManagerRef hid_mgr = 0x0;
static int is_macos_10_10_or_greater = 0;
#if 0
@ -390,6 +394,7 @@ static int init_hid_manager(void)
int HID_API_EXPORT hid_init(void)
{
if (!hid_mgr) {
is_macos_10_10_or_greater = (NSAppKitVersionNumber >= 1343); /* NSAppKitVersionNumber10_10 */
return init_hid_manager();
}
@ -562,11 +567,10 @@ hid_device * HID_API_EXPORT hid_open(unsigned short vendor_id, unsigned short pr
static void hid_device_removal_callback(void *context, IOReturn result,
void *sender)
{
/* Stop the Run Loop for this device. */
hid_device *d = context;
d->disconnected = 1;
CFRunLoopStop(d->run_loop);
/* Stop the Run Loop for this device. This callback will always
be called on the device's registered run loop, so we can just
get the current loop. */
CFRunLoopStop(CFRunLoopGetCurrent());
}
/* The Run Loop calls this function for each input report received.
@ -658,7 +662,7 @@ static void *read_thread(void *param)
while (!dev->shutdown_thread && !dev->disconnected) {
code = CFRunLoopRunInMode(dev->run_loop_mode, 1000/*sec*/, FALSE);
/* Return if the device has been disconnected */
if (code == kCFRunLoopRunFinished) {
if (code == kCFRunLoopRunFinished || code == kCFRunLoopRunStopped) {
dev->disconnected = 1;
break;
}
@ -990,7 +994,7 @@ void HID_API_EXPORT hid_close(hid_device *dev)
return;
/* Disconnect the report callback before close. */
if (!dev->disconnected) {
if (is_macos_10_10_or_greater || !dev->disconnected) {
IOHIDDeviceRegisterInputReportCallback(
dev->device_handle, dev->input_report_buf, dev->max_input_report_len,
NULL, dev);
@ -1014,8 +1018,14 @@ void HID_API_EXPORT hid_close(hid_device *dev)
/* Close the OS handle to the device, but only if it's not
been unplugged. If it's been unplugged, then calling
IOHIDDeviceClose() will crash. */
if (!dev->disconnected) {
IOHIDDeviceClose() will crash.
UPD: The crash part was true in/until some version of macOS.
Starting with macOS 10.15, there is an opposite effect in some environments:
crash happenes if IOHIDDeviceClose() is not called.
Not leaking a resource in all tested environments.
*/
if (is_macos_10_10_or_greater || !dev->disconnected) {
IOHIDDeviceClose(dev->device_handle, kIOHIDOptionsTypeSeizeDevice);
}

View File

@ -280,6 +280,14 @@ void Wiimote::Read()
Report rpt(MAX_PAYLOAD);
auto const result = IORead(rpt.data());
if (0 == result)
{
ERROR_LOG_FMT(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wii Remote {}.", m_index + 1);
DisconnectInternal();
return;
}
// Drop the report if not connected.
if (!m_is_linked)
return;
@ -297,11 +305,6 @@ void Wiimote::Read()
rpt.resize(result);
m_read_reports.Push(std::move(rpt));
}
else if (0 == result)
{
ERROR_LOG_FMT(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wii Remote {}.", m_index + 1);
DisconnectInternal();
}
}
bool Wiimote::Write()