Add linked list of opened HID devices to prevent accessing already freed devices in device removal callback that is sometimes called even after being unregistered
diff --git a/src/hidapi/mac/hid.c b/src/hidapi/mac/hid.c
index d462d26..af34dbf 100644
--- a/src/hidapi/mac/hid.c
+++ b/src/hidapi/mac/hid.c
@@ -121,15 +121,16 @@
pthread_barrier_t barrier; /* Ensures correct startup sequence */
pthread_barrier_t shutdown_barrier; /* Ensures correct shutdown sequence */
int shutdown_thread;
-
- hid_device *next;
};
-/* Static list of all the devices open. This way when a device gets
- disconnected, its hid_device structure can be marked as disconnected
- from hid_device_removal_callback(). */
-static hid_device *device_list = NULL;
-static pthread_mutex_t device_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+struct hid_device_list_node
+{
+ struct hid_device_ *dev;
+ struct hid_device_list_node *next;
+};
+
+static IOHIDManagerRef hid_mgr = 0x0;
+static hid_device_list_node *device_list = 0x0;
static hid_device *new_hid_device(void)
{
@@ -144,7 +145,6 @@
dev->input_report_buf = NULL;
dev->input_reports = NULL;
dev->shutdown_thread = 0;
- dev->next = NULL;
/* Thread objects */
pthread_mutex_init(&dev->mutex, NULL);
@@ -152,22 +152,6 @@
pthread_barrier_init(&dev->barrier, NULL, 2);
pthread_barrier_init(&dev->shutdown_barrier, NULL, 2);
- /* Add the new record to the device_list. */
- pthread_mutex_lock(&device_list_mutex);
- if (!device_list)
- device_list = dev;
- else {
- hid_device *d = device_list;
- while (d) {
- if (!d->next) {
- d->next = dev;
- break;
- }
- d = d->next;
- }
- }
- pthread_mutex_unlock(&device_list_mutex);
-
return dev;
}
@@ -193,6 +177,25 @@
if (dev->source)
CFRelease(dev->source);
free(dev->input_report_buf);
+
+ if (device_list) {
+ if (device_list->dev == dev) {
+ device_list = device_list->next;
+ }
+ else {
+ struct hid_device_list_node *node = device_list;
+ while (node) {
+ if (node->next && node->next->dev == dev) {
+ hid_device_list_node *new_next = node->next->next;
+ free(node->next);
+ node->next = new_next;
+ break;
+ }
+
+ node = node->next;
+ }
+ }
+ }
/* Clean up the thread objects */
pthread_barrier_destroy(&dev->shutdown_barrier);
@@ -200,31 +203,10 @@
pthread_cond_destroy(&dev->condition);
pthread_mutex_destroy(&dev->mutex);
- /* Remove it from the device list. */
- pthread_mutex_lock(&device_list_mutex);
- hid_device *d = device_list;
- if (d == dev) {
- device_list = d->next;
- }
- else {
- while (d) {
- if (d->next == dev) {
- d->next = d->next->next;
- break;
- }
-
- d = d->next;
- }
- }
- pthread_mutex_unlock(&device_list_mutex);
-
/* Free the structure itself. */
free(dev);
}
-static IOHIDManagerRef hid_mgr = 0x0;
-
-
#if 0
static void register_error(hid_device *device, const char *op)
{
@@ -588,20 +570,27 @@
}
static void hid_device_removal_callback(void *context, IOReturn result,
- void *sender, IOHIDDeviceRef dev_ref)
+ void *sender)
{
/* Stop the Run Loop for this device. */
- pthread_mutex_lock(&device_list_mutex);
- hid_device *d = device_list;
- while (d) {
- if (d->device_handle == dev_ref) {
- d->disconnected = 1;
- CFRunLoopStop(d->run_loop);
+ hid_device *dev = (hid_device *)context;
+
+ // The device removal callback is sometimes called even after being
+ // unregistered, leading to a crash when trying to access fields in
+ // the already freed hid_device. We keep a linked list of all created
+ // hid_device's so that the one being removed can be checked against
+ // the list to see if it really hasn't been closed yet and needs to
+ // be dealt with here.
+ hid_device_list_node *node = device_list;
+ while (node) {
+ if (node->dev == dev) {
+ dev->disconnected = 1;
+ CFRunLoopStop(dev->run_loop);
+ break;
}
-
- d = d->next;
+
+ node = node->next;
}
- pthread_mutex_unlock(&device_list_mutex);
}
/* The Run Loop calls this function for each input report received.
@@ -777,8 +766,13 @@
IOHIDDeviceRegisterInputReportCallback(
os_dev, dev->input_report_buf, dev->max_input_report_len,
&hid_report_callback, dev);
- IOHIDManagerRegisterDeviceRemovalCallback(hid_mgr, hid_device_removal_callback, NULL);
-
+ IOHIDDeviceRegisterRemovalCallback(dev->device_handle, hid_device_removal_callback, dev);
+
+ hid_device_list_node *node = (hid_device_list_node *)calloc(1, sizeof(hid_device_list_node));
+ node->dev = dev;
+ node->next = device_list;
+ device_list = node;
+
/* Start the read thread */
pthread_create(&dev->thread, NULL, read_thread, dev);
@@ -1048,7 +1042,7 @@
IOHIDDeviceRegisterInputReportCallback(
dev->device_handle, dev->input_report_buf, dev->max_input_report_len,
NULL, dev);
- IOHIDManagerRegisterDeviceRemovalCallback(hid_mgr, NULL, dev);
+ IOHIDDeviceRegisterRemovalCallback(dev->device_handle, NULL, dev);
IOHIDDeviceUnscheduleFromRunLoop(dev->device_handle, dev->run_loop, dev->run_loop_mode);
IOHIDDeviceScheduleWithRunLoop(dev->device_handle, CFRunLoopGetMain(), kCFRunLoopDefaultMode);
}
