libusb: Fix high CPU usage in callback thread (#783)

Callback_thread used hidapi_thread_cond_timedwait with an absolute
timespec already in the past, causing a tight spin loop. Replaced
with cond_wait + predicate pattern so the thread sleeps until an
event or a shutdown signal arrives.
Multithreading fixes folded in along the way:
- Queue mutex moved from libusb_thread to callback_thread (its
  natural condvar partner), with signal-under-mutex to avoid a
  lost-wakeup race at shutdown.
- process_hotplug_event() now runs under hid_hotplug_context.mutex
  to match hid_hotplug_register_callback()'s HID_API_HOTPLUG_ENUMERATE
  traversal of hid_hotplug_context.devs — fixes a use-after-free
  where a concurrent disconnect could free a list node mid-traversal.
- devs teardown moved to hid_internal_hotplug_cleanup() (after
  hidapi_thread_join) so every access to devs happens under
  hid_hotplug_context.mutex.
- hid_hotplug_register_callback() now frees devs on the
  libusb_hotplug_register_callback() error path, which previously
  leaked.

Closes: #782

Assisted-by: Copilot:claude-sonnet-4.6
Assisted-by: Copilot:claude-opus-4.6
Assisted-by: Claude:claude-opus-4.7

Author: Copilot

.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()
@@ -1143,8 +1151,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) {
@@ -1154,16 +1162,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;
@@ -1204,6 +1217,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);
 
@@ -1216,40 +1231,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;
@@ -1276,8 +1283,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);
 
@@ -1354,8 +1365,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);