Skip to content

Commit

Permalink
Don't leak device handle in macOS 10.10 or newer
Browse files Browse the repository at this point in the history
Merged upstream fix for macOS:
libusb/hidapi@cdc473d

In one of the early versions of macOS, when you try to close the device
with IOHIDDeviceClose() that is being physically disconnected.
Starting with some version of macOS, this crash bug was fixed,
and starting with macSO 10.15 the opposite effect took place:
in some environments crash happens if IOHIDDeviceClose() is _not_ called.

This patch is to keep a workaround for old versions of macOS,
and don't have a leak in new/tested environments.

Fixes libsdl-org#12255
  • Loading branch information
slouken committed Feb 20, 2025
1 parent aa00738 commit 5925c27
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/hidapi/configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ case $host in
backend="mac"
os="darwin"
threads="pthreads"
LIBS="${LIBS} -framework IOKit -framework CoreFoundation"
LIBS="${LIBS} -framework IOKit -framework CoreFoundation -framework AppKit"
;;
*-freebsd*)
AC_MSG_RESULT([ (FreeBSD back-end)])
Expand Down
2 changes: 1 addition & 1 deletion src/hidapi/mac/Makefile-manual
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ COBJS=hid.o
CPPOBJS=../hidtest/hidtest.o
OBJS=$(COBJS) $(CPPOBJS)
CFLAGS+=-I../hidapi -Wall -g -c
LIBS=-framework IOKit -framework CoreFoundation
LIBS=-framework IOKit -framework CoreFoundation -framework AppKit


hidtest: $(OBJS)
Expand Down
21 changes: 17 additions & 4 deletions src/hidapi/mac/hid.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@

#define VALVE_USB_VID 0x28DE

/* 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
Expand Down Expand Up @@ -131,6 +134,7 @@ struct hid_device_list_node
};

static IOHIDManagerRef hid_mgr = 0x0;
static int is_macos_10_10_or_greater = 0;
static struct hid_device_list_node *device_list = 0x0;

static hid_device *new_hid_device(void)
Expand Down Expand Up @@ -485,6 +489,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();
}

Expand Down Expand Up @@ -1138,8 +1143,10 @@ void HID_API_EXPORT hid_close(hid_device *dev)
if (!dev)
return;

/* Disconnect the report callback before close. */
if (!dev->disconnected) {
/* Disconnect the report callback before close.
See comment below.
*/
if (is_macos_10_10_or_greater || !dev->disconnected) {
IOHIDDeviceRegisterInputReportCallback(
dev->device_handle, dev->input_report_buf, dev->max_input_report_len,
NULL, dev);
Expand All @@ -1162,8 +1169,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, kIOHIDOptionsTypeNone);
}

Expand Down

0 comments on commit 5925c27

Please sign in to comment.