diff --git a/.gitignore b/.gitignore index a479f336..ee128dba 100644 --- a/.gitignore +++ b/.gitignore @@ -33,4 +33,6 @@ doxygen/html/ # Visual Studio Code + CMake .vscode/ + +# Local build directory build/ diff --git a/libusb/hid.c b/libusb/hid.c index ad0c4e87..26b8662c 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -172,7 +172,9 @@ static struct hid_hotplug_context { /* Linked list of the hotplug callbacks */ struct hid_hotplug_callback *hotplug_cbs; - /* Linked list of the device infos (mandatory when the device is disconnected) */ + /* Linked list of the device infos (mandatory when the device is disconnected). + * Protected by `mutex` for both reads and writes. + * Final cleanup is done by callback_thread during shutdown. */ struct hid_device_info *devs; } hid_hotplug_context = { .next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE, @@ -1142,8 +1144,8 @@ static int hid_libusb_hotplug_callback(libusb_context *ctx, libusb_device *devic msg->event = event; msg->next = NULL; - /* We use this thread's mutex to protect the queue */ - hidapi_thread_mutex_lock(&hid_hotplug_context.libusb_thread); + /* Use callback_thread's mutex to protect the queue and signal it */ + hidapi_thread_mutex_lock(&hid_hotplug_context.callback_thread); struct hid_hotplug_queue* end = hid_hotplug_context.queue; if (end) { while (end->next) { @@ -1153,16 +1155,21 @@ static int hid_libusb_hotplug_callback(libusb_context *ctx, libusb_device *devic } else { hid_hotplug_context.queue = msg; } - hidapi_thread_mutex_unlock(&hid_hotplug_context.libusb_thread); - /* Wake up the other thread so it can react to the new message immediately */ + /* Wake up the callback thread so it can react to the new message immediately */ hidapi_thread_cond_signal(&hid_hotplug_context.callback_thread); + hidapi_thread_mutex_unlock(&hid_hotplug_context.callback_thread); return 0; } static void process_hotplug_event(struct hid_hotplug_queue* msg) { + /* Lock the mutex to avoid race conditions with hid_hotplug_register_callback(), + * which may iterate devs during HID_API_HOTPLUG_ENUMERATE while holding this mutex. + * The mutex is recursive, so hid_internal_invoke_callbacks() can safely re-acquire it. */ + pthread_mutex_lock(&hid_hotplug_context.mutex); + if (msg->event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED) { struct hid_device_info* info = hid_enumerate_from_libusb(msg->device, 0, 0); struct hid_device_info* info_cur = info; @@ -1203,6 +1210,8 @@ static void process_hotplug_event(struct hid_hotplug_queue* msg) } } + pthread_mutex_unlock(&hid_hotplug_context.mutex); + /* Release the libusb device - we are done with it */ libusb_unref_device(msg->device); @@ -1215,31 +1224,27 @@ static void* callback_thread(void* user_data) { (void) user_data; - /* 5 msec timeout seems reasonable; don't set too low to avoid high CPU usage */ - /* This timeout only affects how much time it takes to stop the thread */ - hidapi_timespec ts; - ts.tv_sec = 0; - ts.tv_nsec = 5000000; - hidapi_thread_mutex_lock(&hid_hotplug_context.callback_thread); - + /* We stop the thread if by the moment there are no events left in the queue there are no callbacks left */ while (1) { - /* Make the tread fall asleep and wait for a condition to wake it up */ - hidapi_thread_cond_timedwait(&hid_hotplug_context.callback_thread, &ts); + /* Wait for events to arrive or shutdown signal */ + while (!hid_hotplug_context.queue && hid_hotplug_context.hotplug_cbs) { + hidapi_thread_cond_wait(&hid_hotplug_context.callback_thread); + } - /* We use this thread's mutex to protect the queue */ - hidapi_thread_mutex_lock(&hid_hotplug_context.libusb_thread); + /* Process all pending events from the queue */ while (hid_hotplug_context.queue) { struct hid_hotplug_queue *cur_event = hid_hotplug_context.queue; - process_hotplug_event(cur_event); + hid_hotplug_context.queue = cur_event->next; - /* Empty the queue */ - cur_event = cur_event->next; - free(hid_hotplug_context.queue); - hid_hotplug_context.queue = cur_event; + /* Release the lock while processing to avoid blocking event producers */ + hidapi_thread_mutex_unlock(&hid_hotplug_context.callback_thread); + process_hotplug_event(cur_event); + free(cur_event); + hidapi_thread_mutex_lock(&hid_hotplug_context.callback_thread); } - hidapi_thread_mutex_unlock(&hid_hotplug_context.libusb_thread); + if (!hid_hotplug_context.hotplug_cbs) { break; } @@ -1275,8 +1280,12 @@ static void* hotplug_thread(void* user_data) libusb_hotplug_deregister_callback(hid_hotplug_context.context, hid_hotplug_context.callback_handle); libusb_exit(hid_hotplug_context.context); - /* Forcibly wake up the thread so it can shut down immediately and wait for it to stop */ + /* hotplug_cbs is already NULL here (the loop above exited because of that). + * Signal callback_thread under the mutex so it can observe the NULL hotplug_cbs + * and exit cleanly, rather than waiting indefinitely in cond_wait. */ + hidapi_thread_mutex_lock(&hid_hotplug_context.callback_thread); hidapi_thread_cond_signal(&hid_hotplug_context.callback_thread); + hidapi_thread_mutex_unlock(&hid_hotplug_context.callback_thread); hidapi_thread_join(&hid_hotplug_context.callback_thread);