libusb: Fix high CPU usage in callback thread

.gitignore

@@ -33,4 +33,6 @@ doxygen/html/
 
 # Visual Studio Code + CMake
 .vscode/
+
+# Local build directory
 build/

libusb/hid.c

@@ -172,7 +172,10 @@ 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`: all reads, writes and the final free during teardown
+	 * are performed while holding it. The teardown free runs in
+	 * hid_internal_hotplug_cleanup() after the hotplug threads have been joined. */
 	struct hid_device_info *devs;
 } hid_hotplug_context = {
 	.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE,
@@ -584,6 +587,11 @@ static void hid_internal_hotplug_cleanup()
 
 	/* Wait for both threads to stop */
 	hidapi_thread_join(&hid_hotplug_context.libusb_thread);
+
+	/* Both hotplug threads have exited: we now have exclusive access to `devs`
+	 * (the caller holds `mutex` and no hotplug event can reach process_hotplug_event). */
+	hid_free_enumeration(hid_hotplug_context.devs);
+	hid_hotplug_context.devs = NULL;
 }
 
 static void hid_internal_hotplug_init()
@@ -1142,8 +1150,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 +1161,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 +1216,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,40 +1230,32 @@ 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;
 		}
 	}
 
-	/* Cleanup connected device list */
-	hid_free_enumeration(hid_hotplug_context.devs);
-	hid_hotplug_context.devs = NULL;
-
 	hidapi_thread_mutex_unlock(&hid_hotplug_context.callback_thread);
 
 	return NULL;
@@ -1275,8 +1282,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);
 
@@ -1353,8 +1364,12 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven
 											LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
 											0, LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY, &hid_libusb_hotplug_callback, NULL,
 											&hid_hotplug_context.callback_handle)) {
-			/* Major malfunction, failed to register a callback */
+			/* Major malfunction, failed to register a callback.
+			 * No hotplug thread was started, so we must unwind `devs` ourselves
+			 * (hid_internal_hotplug_cleanup() would try to join a non-existent thread). */
 			libusb_exit(hid_hotplug_context.context);
+			hid_free_enumeration(hid_hotplug_context.devs);
+			hid_hotplug_context.devs = NULL;
 			free(hotplug_cb);
 			hid_hotplug_context.hotplug_cbs = NULL;
 			pthread_mutex_unlock(&hid_hotplug_context.mutex);