From 99d3ba2136e9111291b1cd63e1d529f1b7219296 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Tue, 14 Jul 2026 01:38:46 +0300 Subject: [PATCH 01/10] linux: deliver the hotplug ENUMERATE pass asynchronously Implements the hotplug contract documented in hidapi.h for the hidraw backend: - hid_hotplug_register_callback() now takes a deep-copied snapshot of the matching connected devices and the udev monitor thread replays it as synthetic "arrived" events: never on an application thread, never from within the register call itself, and always before any live events for that callback; a non-zero return stops the remainder of the pass and deregisters the callback. - Live "arrived" events are delivered as one invocation per device entry with next == NULL (multi-usage devices previously exposed the chained list to callbacks). - All register/deregister failure paths set the global error string and register resets *callback_handle to 0 on failure. - The monitor thread only takes the mutex with trylock and releases no shared state on exit; joining and monitoring-context teardown are centralized in hid_internal_hotplug_cleanup(), fixing an unjoined thread and a teardown race when the last callback removes itself on the monitor thread and a later registration re-creates the context. Assisted-by: claude-code:claude-fable-5 --- linux/hid.c | 272 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 231 insertions(+), 41 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 4d9fa38b2..d989222db 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -933,6 +933,8 @@ static struct hid_hotplug_context { unsigned char mutex_ready; unsigned char mutex_in_use; unsigned char cb_list_dirty; + unsigned char thread_started; + unsigned char replay_pending; /* HIDAPI unique callback handle counter */ hid_hotplug_callback_handle next_handle; @@ -952,6 +954,12 @@ struct hid_hotplug_callback { void *user_data; hid_hotplug_callback_fn callback; + /* Registration-time snapshot of the matching connected devices, + delivered asynchronously by the monitor thread as the + HID_API_HOTPLUG_ENUMERATE initial pass, before any live events + for this callback */ + struct hid_device_info *replay; + /* Pointer to the next notification */ struct hid_hotplug_callback *next; }; @@ -971,6 +979,8 @@ static void hid_internal_hotplug_remove_postponed() struct hid_hotplug_callback *callback = *current; if (!callback->events) { *current = (*current)->next; + /* A deregistered callback never fires again: drop its undelivered snapshot */ + hid_free_enumeration(callback->replay); free(callback); continue; } @@ -990,11 +1000,27 @@ static void hid_internal_hotplug_cleanup() /* Before checking if the list is empty, clear any entries whose removal was postponed first */ hid_internal_hotplug_remove_postponed(); - if (hid_hotplug_context.hotplug_cbs != NULL) { + if (hid_hotplug_context.hotplug_cbs != NULL || !hid_hotplug_context.thread_started) { return; } + /* The monitor thread exits on its own once it observes the empty callback + list; it only ever acquires the mutex with trylock, so joining it while + holding the mutex cannot deadlock */ pthread_join(hid_hotplug_context.thread, NULL); + hid_hotplug_context.thread_started = 0; + + /* The monitor thread releases no shared state on exit (a self-exiting + thread may still be running when a new registration re-creates the + monitoring context): tear it all down here, after the join */ + hid_free_enumeration(hid_hotplug_context.devs); + hid_hotplug_context.devs = NULL; + hid_hotplug_context.replay_pending = 0; + udev_monitor_unref(hid_hotplug_context.mon); + hid_hotplug_context.mon = NULL; + udev_unref(hid_hotplug_context.udev_ctx); + hid_hotplug_context.udev_ctx = NULL; + hid_hotplug_context.monitor_fd = -1; } static void hid_internal_hotplug_init() @@ -1011,6 +1037,8 @@ static void hid_internal_hotplug_init() hid_hotplug_context.mutex_ready = 1; hid_hotplug_context.mutex_in_use = 0; hid_hotplug_context.cb_list_dirty = 0; + hid_hotplug_context.thread_started = 0; + hid_hotplug_context.replay_pending = 0; hid_hotplug_context.monitor_fd = -1; if (hid_hotplug_context.next_handle < FIRST_HOTPLUG_CALLBACK_HANDLE) hid_hotplug_context.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE; @@ -1025,9 +1053,10 @@ static void hid_internal_hotplug_exit() pthread_mutex_lock(&hid_hotplug_context.mutex); struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; - /* Remove all callbacks from the list */ + /* Remove all callbacks from the list, dropping any undelivered snapshots */ while (*current) { struct hid_hotplug_callback* next = (*current)->next; + hid_free_enumeration((*current)->replay); free(*current); *current = next; } @@ -1169,6 +1198,77 @@ void HID_API_EXPORT hid_free_enumeration(struct hid_device_info *devs) } } +/* Deep copy of a single hid_device_info entry; the next pointer of the copy is always NULL */ +static struct hid_device_info *hid_internal_copy_device_info(const struct hid_device_info *src) +{ + struct hid_device_info *dst = (struct hid_device_info*) calloc(1, sizeof(struct hid_device_info)); + if (dst == NULL) { + return NULL; + } + + dst->path = src->path? strdup(src->path): NULL; + dst->vendor_id = src->vendor_id; + dst->product_id = src->product_id; + dst->serial_number = src->serial_number? wcsdup(src->serial_number): NULL; + dst->release_number = src->release_number; + dst->manufacturer_string = src->manufacturer_string? wcsdup(src->manufacturer_string): NULL; + dst->product_string = src->product_string? wcsdup(src->product_string): NULL; + dst->usage_page = src->usage_page; + dst->usage = src->usage; + dst->interface_number = src->interface_number; + dst->next = NULL; + dst->bus_type = src->bus_type; + + return dst; +} + +/* Deliver the registration-time snapshot taken by hid_hotplug_register_callback() + with HID_API_HOTPLUG_ENUMERATE: the initial pass of synthetic "arrived" events. + Only ever runs on the monitor thread, with the mutex held. */ +static void hid_internal_hotplug_replay(struct hid_hotplug_callback *callback) +{ + unsigned char old_state = hid_hotplug_context.mutex_in_use; + hid_hotplug_context.mutex_in_use = 1; + + while (callback->replay) { + /* Detach one entry at a time, so a deregistration from within the callback + (or from another thread, once we return) never sees a dangling list */ + struct hid_device_info *device = callback->replay; + callback->replay = device->next; + device->next = NULL; + if ((*callback->callback)(callback->handle, device, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, callback->user_data)) { + /* A non-zero return deregisters the callback and stops the remainder of the pass */ + callback->events = 0; + hid_hotplug_context.cb_list_dirty = 1; + } + hid_free_enumeration(device); + if (!callback->events) { + /* Deregistered (by return value or from within the callback): drop the undelivered entries */ + hid_free_enumeration(callback->replay); + callback->replay = NULL; + } + } + + hid_hotplug_context.mutex_in_use = old_state; +} + +/* Deliver the pending initial passes of all registered callbacks. + Only ever runs on the monitor thread, with the mutex held (and not in use). */ +static void hid_internal_hotplug_process_replays(void) +{ + /* Clear the flag first: a callback registered during the pass re-sets it, + and an extra scan is harmless */ + hid_hotplug_context.replay_pending = 0; + + for (struct hid_hotplug_callback *callback = hid_hotplug_context.hotplug_cbs; callback; callback = callback->next) { + if (callback->events && callback->replay) { + hid_internal_hotplug_replay(callback); + } + } + + hid_internal_hotplug_remove_postponed(); +} + static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotplug_event event) { pthread_mutex_lock(&hid_hotplug_context.mutex); @@ -1177,6 +1277,11 @@ static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotp struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; while (*current) { struct hid_hotplug_callback *callback = *current; + /* Flush the callback's pending initial pass first, so it never observes + a live event before the synthetic events of HID_API_HOTPLUG_ENUMERATE */ + if (callback->events && callback->replay) { + hid_internal_hotplug_replay(callback); + } if ((callback->events & event) && hid_internal_match_device_id(info->vendor_id, info->product_id, callback->vendor_id, callback->product_id)) { int result = callback->callback(callback->handle, info, event, callback->user_data); @@ -1208,7 +1313,10 @@ static void* hotplug_thread(void* user_data) { (void) user_data; - /* Note: the cleanup sequence is always executed with the mutex locked, so we shoud never lock the mutex without checking if we need to stop */ + /* Note: this thread only ever takes the mutex with trylock: it must never + block on it, so that hid_internal_hotplug_cleanup() can join this thread + while holding the mutex without deadlocking. On a failed trylock, back + off shortly and re-run the loop (poll(NULL, 0, ...) is just a sleep). */ while (hid_hotplug_context.monitor_fd > 0) { fd_set fds; @@ -1221,10 +1329,22 @@ static void* hotplug_thread(void* user_data) break; } + /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE, if any */ + /* NOTE: as above, the flag is read UNLOCKED; it is re-checked (and cleared) under the mutex */ + if (hid_hotplug_context.replay_pending) { + if (pthread_mutex_trylock(&hid_hotplug_context.mutex) != 0) { + poll(NULL, 0, 1); + continue; + } + hid_internal_hotplug_process_replays(); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + continue; + } + FD_ZERO(&fds); FD_SET(hid_hotplug_context.monitor_fd, &fds); /* 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 */ + /* This timeout also caps the latency of the initial HID_API_HOTPLUG_ENUMERATE pass */ tv.tv_sec = 0; tv.tv_usec = 5000; @@ -1237,22 +1357,30 @@ static void* hotplug_thread(void* user_data) /* Check if our file descriptor has received data. */ if (ret > 0 && FD_ISSET(hid_hotplug_context.monitor_fd, &fds)) { + if (pthread_mutex_trylock(&hid_hotplug_context.mutex) != 0) { + /* The udev event stays queued on the netlink socket; retry on the next iteration */ + poll(NULL, 0, 1); + continue; + } /* Make the call to receive the device. select() ensured that this will not block. */ struct udev_device *raw_dev = udev_monitor_receive_device(hid_hotplug_context.mon); if (raw_dev) { - pthread_mutex_lock(&hid_hotplug_context.mutex); const char* action = udev_device_get_action(raw_dev); if (!strcmp(action, "add")) { - // We create a list of all usages on this UDEV device + /* We create a list of all usages on this UDEV device */ struct hid_device_info *info = create_device_info_for_device(raw_dev); struct hid_device_info *info_cur = info; while (info_cur) { - /* For each device, call all matching callbacks */ - /* TODO: possibly make the `next` field NULL to match the behavior on other systems */ + /* For each device, call all matching callbacks, one entry + per invocation: detach the entry for the duration of the + call, so the callback always sees next == NULL */ + struct hid_device_info *info_next = info_cur->next; + info_cur->next = NULL; hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED); - info_cur = info_cur->next; + info_cur->next = info_next; + info_cur = info_next; } /* Append all we got to the end of the device list */ @@ -1283,17 +1411,15 @@ static void* hotplug_thread(void* user_data) } } udev_device_unref(raw_dev); - pthread_mutex_unlock(&hid_hotplug_context.mutex); } + pthread_mutex_unlock(&hid_hotplug_context.mutex); } } - /* Cleanup connected device list */ - hid_free_enumeration(hid_hotplug_context.devs); - hid_hotplug_context.devs = NULL; - /* Disarm the udev monitor */ - udev_monitor_unref(hid_hotplug_context.mon); - udev_unref(hid_hotplug_context.udev_ctx); + /* All shared state (the device list, the udev monitor and context) is + released by whoever joins this thread (hid_internal_hotplug_cleanup): + doing it here would race against hid_hotplug_register_callback() + re-creating the monitoring context while this thread is still exiting */ return NULL; } @@ -1302,17 +1428,33 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven { struct hid_hotplug_callback* hotplug_cb; + /* No events can be delivered before the out parameter is written */ + if (callback_handle != NULL) { + *callback_handle = 0; + } + /* Check params */ + if (callback == NULL) { + register_global_error("Hotplug callback function is NULL"); + return -1; + } if (events == 0 - || (events & ~(HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED | HID_API_HOTPLUG_EVENT_DEVICE_LEFT)) - || (flags & ~(HID_API_HOTPLUG_ENUMERATE)) - || callback == NULL) { + || (events & ~(HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED | HID_API_HOTPLUG_EVENT_DEVICE_LEFT))) { + register_global_error("Hotplug events mask contains no valid events or unknown bits"); return -1; } + if (flags & ~(HID_API_HOTPLUG_ENUMERATE)) { + register_global_error("Hotplug flags mask contains unknown bits"); + return -1; + } + + hid_init(); + /* register_global_error: global error is reset by hid_init */ hotplug_cb = (struct hid_hotplug_callback*)calloc(1, sizeof(struct hid_hotplug_callback)); if (hotplug_cb == NULL) { + register_global_error("Failed to allocate a hotplug callback record"); return -1; } @@ -1323,6 +1465,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hotplug_cb->events = events; hotplug_cb->user_data = user_data; hotplug_cb->callback = callback; + hotplug_cb->replay = NULL; /* Ensure we are ready to actually use the mutex */ hid_internal_hotplug_init(); @@ -1338,11 +1481,6 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hid_hotplug_context.next_handle = 1; } - /* Return allocated handle */ - if (callback_handle != NULL) { - *callback_handle = hotplug_cb->handle; - } - /* Append a new callback to the end */ if (hid_hotplug_context.hotplug_cbs != NULL) { struct hid_hotplug_callback *last = hid_hotplug_context.hotplug_cbs; @@ -1352,57 +1490,102 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven last->next = hotplug_cb; } else { - // Prepare a UDEV context to run monitoring on + /* Reap the monitor thread first in case it is exiting (or has exited) + after the removal of its last callback, and release the previous + monitoring context with it */ + hid_internal_hotplug_cleanup(); + + /* Prepare a UDEV context to run monitoring on */ hid_hotplug_context.udev_ctx = udev_new(); if (!hid_hotplug_context.udev_ctx) { pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Couldn't create udev context"); return -1; } hid_hotplug_context.mon = udev_monitor_new_from_netlink(hid_hotplug_context.udev_ctx, "udev"); + if (!hid_hotplug_context.mon) { + udev_unref(hid_hotplug_context.udev_ctx); + hid_hotplug_context.udev_ctx = NULL; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Couldn't create udev monitor"); + return -1; + } udev_monitor_filter_add_match_subsystem_devtype(hid_hotplug_context.mon, "hidraw", NULL); udev_monitor_enable_receiving(hid_hotplug_context.mon); hid_hotplug_context.monitor_fd = udev_monitor_get_fd(hid_hotplug_context.mon); /* After monitoring is all set up, enumerate all devices */ hid_hotplug_context.devs = hid_enumerate(0, 0); + register_global_error(NULL); /* an empty system is not a registration failure */ /* Don't forget to actually register the callback */ hid_hotplug_context.hotplug_cbs = hotplug_cb; /* Start the thread that will be doing the event scanning */ - pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL); + if (pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL) != 0) { + hid_hotplug_context.hotplug_cbs = NULL; + hid_free_enumeration(hid_hotplug_context.devs); + hid_hotplug_context.devs = NULL; + udev_monitor_unref(hid_hotplug_context.mon); + hid_hotplug_context.mon = NULL; + udev_unref(hid_hotplug_context.udev_ctx); + hid_hotplug_context.udev_ctx = NULL; + hid_hotplug_context.monitor_fd = -1; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Couldn't create the hotplug monitor thread"); + return -1; + } + hid_hotplug_context.thread_started = 1; + } + + /* Return allocated handle */ + if (callback_handle != NULL) { + *callback_handle = hotplug_cb->handle; } - /* Mark the mutex as IN USE, to prevent callback removal from inside a callback */ - unsigned char old_state = hid_hotplug_context.mutex_in_use; - hid_hotplug_context.mutex_in_use = 1; - if ((flags & HID_API_HOTPLUG_ENUMERATE) && (events & HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED)) { - struct hid_device_info* device = hid_hotplug_context.devs; - /* Notify about already connected devices, if asked so */ - while (device != NULL) { - if (hid_internal_match_device_id(device->vendor_id, device->product_id, hotplug_cb->vendor_id, hotplug_cb->product_id)) { - (*hotplug_cb->callback)(hotplug_cb->handle, device, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, hotplug_cb->user_data); + /* Take a snapshot of the matching connected devices: the monitor thread + delivers it asynchronously as the initial pass of synthetic "arrived" + events, before any live events for this callback and never from + within this call */ + struct hid_device_info **replay_tail = &hotplug_cb->replay; + for (struct hid_device_info *device = hid_hotplug_context.devs; device != NULL; device = device->next) { + if (!hid_internal_match_device_id(device->vendor_id, device->product_id, hotplug_cb->vendor_id, hotplug_cb->product_id)) { + continue; } - - device = device->next; + *replay_tail = hid_internal_copy_device_info(device); + if (*replay_tail == NULL) { + /* Out of memory: deliver as much of the snapshot as we could copy */ + break; + } + replay_tail = &(*replay_tail)->next; + } + if (hotplug_cb->replay != NULL) { + hid_hotplug_context.replay_pending = 1; } } - hid_hotplug_context.mutex_in_use = old_state; - hid_internal_hotplug_cleanup(); pthread_mutex_unlock(&hid_hotplug_context.mutex); - + return 0; } int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_callback_handle callback_handle) { - if (!hid_hotplug_context.mutex_ready || callback_handle <= 0) { + if (callback_handle <= 0) { + register_global_error("Invalid hotplug callback handle"); + return -1; + } + + if (!hid_hotplug_context.mutex_ready) { + register_global_error("No hotplug callbacks are registered"); return -1; } @@ -1410,6 +1593,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_call if (hid_hotplug_context.hotplug_cbs == NULL) { pthread_mutex_unlock(&hid_hotplug_context.mutex); + register_global_error("No hotplug callbacks are registered"); return -1; } @@ -1424,6 +1608,8 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_call hid_hotplug_context.cb_list_dirty = 1; } else { struct hid_hotplug_callback *next = (*current)->next; + /* A deregistered callback never fires again: drop its undelivered snapshot */ + hid_free_enumeration((*current)->replay); free(*current); *current = next; } @@ -1436,6 +1622,10 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_call pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (result < 0) { + register_global_error("Hotplug callback handle not found"); + } + return result; } From b447616cd21d9785cc8f861934cbe6fe293bf55f Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Tue, 14 Jul 2026 20:18:49 +0300 Subject: [PATCH 02/10] linux: address the hotplug review findings Follow-up to the asynchronous ENUMERATE pass, fixing the issues raised in the first review round: - Serialize every mutation of the error strings with a dedicated mutex: registration/deregistration and the monitor thread could write (and double-free) the global error string concurrently. - Guard the first-time initialization of the hotplug machinery with a static bootstrap mutex: two concurrent first registrations could both initialize the mutex and reset the context fields. - Serialize hid_init(): a registration implicitly initializes the library both directly and through its initial enumeration, and setlocale() is not thread-safe against itself. - Never read the monitor loop's decision state unlocked, and never join the monitor thread while holding the mutex: the thread announces its exit under the mutex, and the cleanup unlocks before joining. - Skip devices that are already known on arrival: a device showing up between arming the udev monitor and taking the initial enumeration was reported (and cached) twice, and left twice. - Update the device cache before dispatching an event, and bound every dispatch to the callbacks registered when the event started, so that a callback registered from within a callback observes the device through its own snapshot exactly once, instead of missing it or seeing a "left" with no matching "arrived". - Make the initial snapshot all-or-nothing: a failed copy now unwinds the registration instead of delivering a partial device list, and a genuine enumeration failure fails the registration (an empty system does not). - Check every libudev setup call and fail the registration with an error string; treat only a negative monitor file descriptor as invalid (0 is a valid one). - Fail the registration when the callback handles are exhausted instead of overflowing (undefined behaviour) and wrapping onto live handles. - Report deregistration of an already deregistered handle as not found. - Reap a self-exited monitor thread before deregistration returns early. - Handle a NULL action/devnode from udev, and use poll() instead of select() for the monitor file descriptor. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 622 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 439 insertions(+), 183 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index d989222db..000fa7489 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -26,6 +26,7 @@ #include #include #include +#include /* Unix */ #include #include @@ -135,14 +136,24 @@ static wchar_t *utf8_to_wchar_t(const char *utf8) } +/* Serializes concurrent mutations of the error strings: the hotplug API is + * thread-safe and may set the global error from several threads at once + * (including the hotplug monitor thread). Reading the strings via hid_error() + * remains subject to the documented thread-safety rules. */ +static pthread_mutex_t error_str_mutex = PTHREAD_MUTEX_INITIALIZER; + /* Makes a copy of the given error message (and decoded according to the * currently locale) into the wide string pointer pointed by error_str. * The last stored error string is freed. * Use register_error_str(NULL) to free the error message completely. */ static void register_error_str(wchar_t **error_str, const char *msg) { + wchar_t *new_str = utf8_to_wchar_t(msg); + + pthread_mutex_lock(&error_str_mutex); free(*error_str); - *error_str = utf8_to_wchar_t(msg); + *error_str = new_str; + pthread_mutex_unlock(&error_str_mutex); } /* Semilar to register_error_str, but allows passing a format string with va_list args into this function. */ @@ -914,6 +925,18 @@ HID_API_EXPORT const char* HID_API_CALL hid_version_str(void) return HID_API_VERSION_STR; } +/* Lifecycle of the udev monitor thread; every transition happens under the + hotplug mutex */ +enum hid_hotplug_thread_state { + /* No monitor thread, nothing to join */ + HID_HOTPLUG_THREAD_NONE, + /* The monitor thread is running */ + HID_HOTPLUG_THREAD_RUNNING, + /* The monitor thread has announced its exit and no longer touches any + shared state; it awaits pthread_join */ + HID_HOTPLUG_THREAD_FINISHED +}; + static struct hid_hotplug_context { /* UDEV context that handles the monitor */ struct udev* udev_ctx; @@ -921,20 +944,20 @@ static struct hid_hotplug_context { /* UDEV monitor that receives events */ struct udev_monitor* mon; - /* File descriptor for the UDEV monitor that allows to check for new events with select() */ + /* File descriptor for the UDEV monitor that allows to check for new events with poll() */ int monitor_fd; /* Thread for the UDEV monitor */ pthread_t thread; + enum hid_hotplug_thread_state thread_state; + pthread_mutex_t mutex; /* Boolean flags */ unsigned char mutex_ready; unsigned char mutex_in_use; unsigned char cb_list_dirty; - unsigned char thread_started; - unsigned char replay_pending; /* HIDAPI unique callback handle counter */ hid_hotplug_callback_handle next_handle; @@ -991,40 +1014,78 @@ static void hid_internal_hotplug_remove_postponed() hid_hotplug_context.cb_list_dirty = 0; } +/* Releases a (possibly partially initialized) monitoring context. + Called with the mutex held, only when the monitor thread cannot touch it + anymore: before it is started, or after it has been joined. */ +static void hid_internal_hotplug_release_monitor(void) +{ + hid_free_enumeration(hid_hotplug_context.devs); + hid_hotplug_context.devs = NULL; + if (hid_hotplug_context.mon) { + udev_monitor_unref(hid_hotplug_context.mon); + hid_hotplug_context.mon = NULL; + } + if (hid_hotplug_context.udev_ctx) { + udev_unref(hid_hotplug_context.udev_ctx); + hid_hotplug_context.udev_ctx = NULL; + } + hid_hotplug_context.monitor_fd = -1; +} + +/* Reaps the monitor thread and releases the monitoring context once the last + callback is gone. Called with the mutex held exactly once (and not in use); + the mutex is temporarily dropped while waiting for/joining the thread, so + the caller must re-validate any cached state after this returns. */ static void hid_internal_hotplug_cleanup() { if (!hid_hotplug_context.mutex_ready || hid_hotplug_context.mutex_in_use) { return; } - /* Before checking if the list is empty, clear any entries whose removal was postponed first */ - hid_internal_hotplug_remove_postponed(); + for (;;) { + pthread_t thread; - if (hid_hotplug_context.hotplug_cbs != NULL || !hid_hotplug_context.thread_started) { - return; - } + /* Before checking if the list is empty, clear any entries whose removal was postponed first */ + hid_internal_hotplug_remove_postponed(); - /* The monitor thread exits on its own once it observes the empty callback - list; it only ever acquires the mutex with trylock, so joining it while - holding the mutex cannot deadlock */ - pthread_join(hid_hotplug_context.thread, NULL); - hid_hotplug_context.thread_started = 0; + if (hid_hotplug_context.hotplug_cbs != NULL || hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { + /* Still serving callbacks, no thread to reap, or a concurrent + caller reaped it already (and possibly started a new context) */ + return; + } - /* The monitor thread releases no shared state on exit (a self-exiting - thread may still be running when a new registration re-creates the - monitoring context): tear it all down here, after the join */ - hid_free_enumeration(hid_hotplug_context.devs); - hid_hotplug_context.devs = NULL; - hid_hotplug_context.replay_pending = 0; - udev_monitor_unref(hid_hotplug_context.mon); - hid_hotplug_context.mon = NULL; - udev_unref(hid_hotplug_context.udev_ctx); - hid_hotplug_context.udev_ctx = NULL; - hid_hotplug_context.monitor_fd = -1; + if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { + /* Claim the join, so no concurrent caller joins the thread twice */ + thread = hid_hotplug_context.thread; + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + + /* The thread no longer touches the shared state once it has + announced itself FINISHED (under this mutex) */ + hid_internal_hotplug_release_monitor(); + + /* Never block in pthread_join while holding the mutex */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + pthread_join(thread, NULL); + pthread_mutex_lock(&hid_hotplug_context.mutex); + return; + } + + /* HID_HOTPLUG_THREAD_RUNNING: drop the mutex so the thread can acquire + it, observe the empty callback list and announce its exit, then + re-evaluate from scratch */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + poll(NULL, 0, 1); + pthread_mutex_lock(&hid_hotplug_context.mutex); + } } +/* Protects the first-time initialization of the hotplug machinery (and its + teardown in hid_exit) against concurrent hotplug registrations */ +static pthread_mutex_t hid_hotplug_startup_mutex = PTHREAD_MUTEX_INITIALIZER; + static void hid_internal_hotplug_init() { + pthread_mutex_lock(&hid_hotplug_startup_mutex); if (!hid_hotplug_context.mutex_ready) { /* Initialize the mutex as recursive */ pthread_mutexattr_t attr; @@ -1034,20 +1095,31 @@ static void hid_internal_hotplug_init() pthread_mutexattr_destroy(&attr); /* Set state to Ready */ - hid_hotplug_context.mutex_ready = 1; hid_hotplug_context.mutex_in_use = 0; hid_hotplug_context.cb_list_dirty = 0; - hid_hotplug_context.thread_started = 0; - hid_hotplug_context.replay_pending = 0; + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; hid_hotplug_context.monitor_fd = -1; + /* The handles are monotonic and not reused while the library remains + initialized (see hidapi.h); the counter survives hid_exit */ if (hid_hotplug_context.next_handle < FIRST_HOTPLUG_CALLBACK_HANDLE) hid_hotplug_context.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE; + hid_hotplug_context.mutex_ready = 1; } + pthread_mutex_unlock(&hid_hotplug_startup_mutex); } static void hid_internal_hotplug_exit() { - if (!hid_hotplug_context.mutex_ready) { + unsigned char machinery_ready; + + /* Never hold the startup mutex while waiting for the hotplug mutex: the + monitor thread takes the startup mutex from within callbacks (nested + register/deregister calls) while holding the hotplug mutex */ + pthread_mutex_lock(&hid_hotplug_startup_mutex); + machinery_ready = hid_hotplug_context.mutex_ready; + pthread_mutex_unlock(&hid_hotplug_startup_mutex); + + if (!machinery_ready) { return; } @@ -1060,12 +1132,26 @@ static void hid_internal_hotplug_exit() free(*current); *current = next; } + /* Reap the monitor thread (temporarily dropping the mutex) and release the monitoring context */ hid_internal_hotplug_cleanup(); pthread_mutex_unlock(&hid_hotplug_context.mutex); + + /* The monitor thread is joined by now and hid_exit is (per the API + thread-safety rules) serialized against every other HIDAPI call except + hid_hotplug_(de)register_callback on other threads - which require the + machinery to be observed ready first */ + pthread_mutex_lock(&hid_hotplug_startup_mutex); hid_hotplug_context.mutex_ready = 0; pthread_mutex_destroy(&hid_hotplug_context.mutex); + pthread_mutex_unlock(&hid_hotplug_startup_mutex); } +/* Serializes the implicit initialization: the hotplug API allows concurrent + hid_hotplug_register_callback() calls, each of which implicitly initializes + the library - directly, and once more through the initial enumeration - and + setlocale() is not thread-safe against itself. */ +static pthread_mutex_t hid_init_mutex = PTHREAD_MUTEX_INITIALIZER; + int HID_API_EXPORT hid_init(void) { const char *locale; @@ -1073,10 +1159,12 @@ int HID_API_EXPORT hid_init(void) /* indicate no error */ register_global_error(NULL); + pthread_mutex_lock(&hid_init_mutex); /* Set the locale if it's not set. */ locale = setlocale(LC_CTYPE, NULL); if (!locale) setlocale(LC_CTYPE, ""); + pthread_mutex_unlock(&hid_init_mutex); return 0; } @@ -1097,7 +1185,10 @@ static int hid_internal_match_device_id(unsigned short vendor_id, unsigned short return (expected_vendor_id == 0x0 || vendor_id == expected_vendor_id) && (expected_product_id == 0x0 || product_id == expected_product_id); } -struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, unsigned short product_id) +/* Same as hid_enumerate, but distinguishes a genuine failure from an empty + system: *failure (when non-NULL) is set to 1 only when the enumeration + itself failed (with the global error set accordingly). */ +static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, unsigned short product_id, int *failure) { struct udev *udev; struct udev_enumerate *enumerate; @@ -1106,6 +1197,10 @@ struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, struct hid_device_info *root = NULL; /* return object */ struct hid_device_info *cur_dev = NULL; + if (failure) { + *failure = 0; + } + hid_init(); /* register_global_error: global error is reset by hid_init */ @@ -1113,11 +1208,22 @@ struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, udev = udev_new(); if (!udev) { register_global_error("Couldn't create udev context"); + if (failure) { + *failure = 1; + } return NULL; } /* Create a list of the devices in the 'hidraw' subsystem. */ enumerate = udev_enumerate_new(udev); + if (!enumerate) { + udev_unref(udev); + register_global_error("Couldn't create udev enumeration"); + if (failure) { + *failure = 1; + } + return NULL; + } udev_enumerate_add_match_subsystem(enumerate, "hidraw"); udev_enumerate_scan_devices(enumerate); devices = udev_enumerate_get_list_entry(enumerate); @@ -1184,6 +1290,11 @@ struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, return root; } +struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, unsigned short product_id) +{ + return hid_internal_enumerate(vendor_id, product_id, NULL); +} + void HID_API_EXPORT hid_free_enumeration(struct hid_device_info *devs) { struct hid_device_info *d = devs; @@ -1219,6 +1330,15 @@ static struct hid_device_info *hid_internal_copy_device_info(const struct hid_de dst->next = NULL; dst->bus_type = src->bus_type; + /* A partial copy must never reach a callback */ + if ((src->path && !dst->path) + || (src->serial_number && !dst->serial_number) + || (src->manufacturer_string && !dst->manufacturer_string) + || (src->product_string && !dst->product_string)) { + hid_free_enumeration(dst); + return NULL; + } + return dst; } @@ -1256,10 +1376,6 @@ static void hid_internal_hotplug_replay(struct hid_hotplug_callback *callback) Only ever runs on the monitor thread, with the mutex held (and not in use). */ static void hid_internal_hotplug_process_replays(void) { - /* Clear the flag first: a callback registered during the pass re-sets it, - and an extra scan is harmless */ - hid_hotplug_context.replay_pending = 0; - for (struct hid_hotplug_callback *callback = hid_hotplug_context.hotplug_cbs; callback; callback = callback->next) { if (callback->events && callback->replay) { hid_internal_hotplug_replay(callback); @@ -1269,14 +1385,20 @@ static void hid_internal_hotplug_process_replays(void) hid_internal_hotplug_remove_postponed(); } -static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotplug_event event) +/* Deliver one live event to the matching callbacks whose handle does not + exceed dispatch_bound. The bound freezes the dispatch to the callbacks + registered before the event started: a callback registered from within + a callback observes the device through its registration snapshot instead + of the in-flight event, keeping the arrivals exactly-once (the handles + are monotonic and the list is kept in registration order). */ +static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotplug_event event, hid_hotplug_callback_handle dispatch_bound) { pthread_mutex_lock(&hid_hotplug_context.mutex); hid_hotplug_context.mutex_in_use = 1; - struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; - while (*current) { - struct hid_hotplug_callback *callback = *current; + for (struct hid_hotplug_callback *callback = hid_hotplug_context.hotplug_cbs; + callback != NULL && callback->handle <= dispatch_bound; + callback = callback->next) { /* Flush the callback's pending initial pass first, so it never observes a live event before the synthetic events of HID_API_HOTPLUG_ENUMERATE */ if (callback->events && callback->replay) { @@ -1287,12 +1409,10 @@ static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotp int result = callback->callback(callback->handle, info, event, callback->user_data); /* If the result is non-zero, we mark the callback for removal and proceed */ if (result) { - (*current)->events = 0; + callback->events = 0; hid_hotplug_context.cb_list_dirty = 1; - continue; } } - current = &callback->next; } hid_hotplug_context.mutex_in_use = 0; @@ -1300,126 +1420,207 @@ static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotp pthread_mutex_unlock(&hid_hotplug_context.mutex); } -static int match_udev_to_info(struct udev_device* raw_dev, struct hid_device_info *info) +static struct hid_device_info *hid_internal_find_device_by_path(const char *path) { - const char *path = udev_device_get_devnode(raw_dev); - if (!strcmp(path, info->path)) { - return 1; + if (path == NULL) { + return NULL; } - return 0; + for (struct hid_device_info *device = hid_hotplug_context.devs; device; device = device->next) { + if (device->path && !strcmp(device->path, path)) { + return device; + } + } + return NULL; } -static void* hotplug_thread(void* user_data) +/* Handle a device arrival: update the connected-device cache and dispatch the + callbacks. Takes ownership of the whole device chain (one entry per usage). + Only ever runs on the monitor thread, with the mutex held. */ +static void hid_internal_hotplug_process_arrival(struct hid_device_info *info) { - (void) user_data; + hid_hotplug_callback_handle dispatch_bound; - /* Note: this thread only ever takes the mutex with trylock: it must never - block on it, so that hid_internal_hotplug_cleanup() can join this thread - while holding the mutex without deadlocking. On a failed trylock, back - off shortly and re-run the loop (poll(NULL, 0, ...) is just a sleep). */ + if (info == NULL) { + return; + } - while (hid_hotplug_context.monitor_fd > 0) { - fd_set fds; - struct timeval tv; - int ret; + /* The device may already be known: a device arriving between arming the + udev monitor and taking the initial enumeration at first registration + is both in the cache and queued as an event on the monitor socket. + Skip duplicates to keep arrivals (and later departures) exactly-once. */ + if (hid_internal_find_device_by_path(info->path) != NULL) { + hid_free_enumeration(info); + return; + } - /* On every iteration, check if we still have any callbacks left and leave if none are left */ - /* NOTE: the check is performed UNLOCKED and the value CAN change in the background */ - if (!hid_hotplug_context.hotplug_cbs) { - break; + /* Append to the cache BEFORE dispatching, so an ENUMERATE snapshot taken + by a callback registered from within a callback captures the whole + device rather than the in-flight event */ + if (hid_hotplug_context.devs != NULL) { + struct hid_device_info *last = hid_hotplug_context.devs; + while (last->next != NULL) { + last = last->next; } + last->next = info; + } else { + hid_hotplug_context.devs = info; + } - /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE, if any */ - /* NOTE: as above, the flag is read UNLOCKED; it is re-checked (and cleared) under the mutex */ - if (hid_hotplug_context.replay_pending) { - if (pthread_mutex_trylock(&hid_hotplug_context.mutex) != 0) { - poll(NULL, 0, 1); - continue; - } - hid_internal_hotplug_process_replays(); - pthread_mutex_unlock(&hid_hotplug_context.mutex); - continue; + /* Freeze the dispatch to the callbacks registered up to this point */ + dispatch_bound = hid_hotplug_context.next_handle - 1; + + for (struct hid_device_info *info_cur = info; info_cur; info_cur = info_cur->next) { + /* One usage entry per invocation: the callback must always see + device->next == NULL, and a copy keeps the cache walkable for the + snapshots of callbacks registered from within a callback */ + struct hid_device_info *copy = hid_internal_copy_device_info(info_cur); + if (copy != NULL) { + hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); + hid_free_enumeration(copy); + } else { + /* Out of memory: deliver the cache entry itself, temporarily + detached from the list */ + struct hid_device_info *info_next = info_cur->next; + info_cur->next = NULL; + hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); + info_cur->next = info_next; + } + } +} + +/* Handle a device removal: detach the matching entries from the + connected-device cache and dispatch the callbacks. + Only ever runs on the monitor thread, with the mutex held. */ +static void hid_internal_hotplug_process_removal(const char *devnode) +{ + /* Freeze the dispatch to the callbacks registered up to this point + (see hid_internal_hotplug_process_arrival) */ + hid_hotplug_callback_handle dispatch_bound = hid_hotplug_context.next_handle - 1; + struct hid_device_info *removed = NULL; + struct hid_device_info **removed_tail = &removed; + + /* Detach every usage entry of the device from the cache BEFORE dispatching + any of them: a callback registered from within this dispatch is excluded + from it by the bound, so it must not be able to capture a still-cached + usage entry of the leaving device in its ENUMERATE snapshot either - + that would be an "arrived" without a matching "left" */ + for (struct hid_device_info **current = &hid_hotplug_context.devs; *current;) { + struct hid_device_info *info = *current; + if (info->path && !strcmp(devnode, info->path)) { + *current = info->next; + info->next = NULL; + *removed_tail = info; + removed_tail = &info->next; + } else { + current = &info->next; } + } - FD_ZERO(&fds); - FD_SET(hid_hotplug_context.monitor_fd, &fds); - /* 5 msec timeout seems reasonable; don't set too low to avoid high CPU usage */ - /* This timeout also caps the latency of the initial HID_API_HOTPLUG_ENUMERATE pass */ - tv.tv_sec = 0; - tv.tv_usec = 5000; + while (removed != NULL) { + struct hid_device_info *info = removed; + removed = info->next; + /* One usage entry per invocation: the callback always sees next == NULL */ + info->next = NULL; + hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); + /* Free every removed device */ + hid_free_enumeration(info); + } +} - ret = select(hid_hotplug_context.monitor_fd+1, &fds, NULL, NULL, &tv); +/* Dispatch one udev monitor event. + Only ever runs on the monitor thread, with the mutex held. */ +static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) +{ + const char *action = udev_device_get_action(raw_dev); + if (action == NULL) { + return; + } - /* An extra check, just in case within those 5msec the thread was told to stop */ - if (!hid_hotplug_context.hotplug_cbs) { - break; + if (!strcmp(action, "add")) { + /* We create a list of all usages on this UDEV device */ + hid_internal_hotplug_process_arrival(create_device_info_for_device(raw_dev)); + } else if (!strcmp(action, "remove")) { + const char *devnode = udev_device_get_devnode(raw_dev); + if (devnode != NULL) { + hid_internal_hotplug_process_removal(devnode); } + } +} - /* Check if our file descriptor has received data. */ - if (ret > 0 && FD_ISSET(hid_hotplug_context.monitor_fd, &fds)) { - if (pthread_mutex_trylock(&hid_hotplug_context.mutex) != 0) { - /* The udev event stays queued on the netlink socket; retry on the next iteration */ - poll(NULL, 0, 1); - continue; - } +static void* hotplug_thread(void* user_data) +{ + /* The monitor file descriptor is immutable while this thread runs: it is + set up before the thread starts and released only after it is joined */ + const int monitor_fd = hid_hotplug_context.monitor_fd; + + (void) user_data; - /* Make the call to receive the device. - select() ensured that this will not block. */ - struct udev_device *raw_dev = udev_monitor_receive_device(hid_hotplug_context.mon); - if (raw_dev) { - const char* action = udev_device_get_action(raw_dev); - if (!strcmp(action, "add")) { - /* We create a list of all usages on this UDEV device */ - struct hid_device_info *info = create_device_info_for_device(raw_dev); - struct hid_device_info *info_cur = info; - while (info_cur) { - /* For each device, call all matching callbacks, one entry - per invocation: detach the entry for the duration of the - call, so the callback always sees next == NULL */ - struct hid_device_info *info_next = info_cur->next; - info_cur->next = NULL; - hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED); - info_cur->next = info_next; - info_cur = info_next; - } - - /* Append all we got to the end of the device list */ - if (info) { - if (hid_hotplug_context.devs != NULL) { - struct hid_device_info *last = hid_hotplug_context.devs; - while (last->next != NULL) { - last = last->next; - } - last->next = info; - } else { - hid_hotplug_context.devs = info; - } - } - } else if (!strcmp(action, "remove")) { - for (struct hid_device_info **current = &hid_hotplug_context.devs; *current;) { - struct hid_device_info* info = *current; - if (match_udev_to_info(raw_dev, *current)) { - /* If the libusb device that's left matches this HID device, we detach it from the list */ - *current = (*current)->next; - info->next = NULL; - hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT); - /* Free every removed device */ - hid_free_enumeration(info); - } else { - current = &info->next; - } - } + /* This thread takes the mutex with trylock only: it must never block on + the mutex, so that it always makes progress towards its exit check + while hid_internal_hotplug_cleanup() waits, without holding the mutex, + for it to announce its own exit. All shared state, including the loop + decisions, is only ever accessed with the mutex held. */ + + for (;;) { + struct pollfd fds; + int ret; + int stop = 0; + + if (pthread_mutex_trylock(&hid_hotplug_context.mutex) != 0) { + /* Contended: back off shortly and retry. Polling the monitor fd + here would spin, as it stays readable until the queued events + are drained (which needs the mutex). */ + poll(NULL, 0, 1); + continue; + } + + if (hid_hotplug_context.hotplug_cbs == NULL) { + /* The last callback is gone: announce the exit under the mutex, + then stop touching any shared state */ + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_FINISHED; + stop = 1; + } else { + /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE + before any queued live events */ + hid_internal_hotplug_process_replays(); + + /* Drain and dispatch the events queued on the (non-blocking) + udev monitor socket */ + for (;;) { + struct udev_device *raw_dev = udev_monitor_receive_device(hid_hotplug_context.mon); + if (raw_dev == NULL) { + break; } + hid_internal_hotplug_process_event(raw_dev); udev_device_unref(raw_dev); } - pthread_mutex_unlock(&hid_hotplug_context.mutex); + } + + pthread_mutex_unlock(&hid_hotplug_context.mutex); + + if (stop) { + break; + } + + /* Wait for udev events; the timeout paces the mutex retries and caps + the latency of the initial HID_API_HOTPLUG_ENUMERATE passes. + 5 msec seems reasonable; don't set too low to avoid high CPU usage. */ + fds.fd = monitor_fd; + fds.events = POLLIN; + fds.revents = 0; + ret = poll(&fds, 1, 5); + if (ret > 0 && !(fds.revents & POLLIN)) { + /* Monitor socket failure: without this, a dead socket would make + poll() return immediately and busy-spin this loop; keep the + 5 msec pace instead (events are lost either way) */ + poll(NULL, 0, 5); } } - /* All shared state (the device list, the udev monitor and context) is - released by whoever joins this thread (hid_internal_hotplug_cleanup): - doing it here would race against hid_hotplug_register_callback() - re-creating the monitoring context while this thread is still exiting */ + /* The device cache, the udev monitor and its context stay allocated until + hid_internal_hotplug_cleanup() joins this thread (on a later hotplug + registration/deregistration or at hid_exit): releasing them here would + race a re-created monitoring context */ return NULL; } @@ -1448,6 +1649,8 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven return -1; } + /* Implicit hid_init: unlike the other API entry points, concurrent + registrations are allowed - hid_init() serializes itself */ hid_init(); /* register_global_error: global error is reset by hid_init */ @@ -1473,13 +1676,20 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven /* Lock the mutex to avoid race conditions */ pthread_mutex_lock(&hid_hotplug_context.mutex); + /* The handles are monotonic and never reused: fail instead of overflowing */ + if (hid_hotplug_context.next_handle == INT_MAX) { + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Hotplug callback handles exhausted"); + return -1; + } + hotplug_cb->handle = hid_hotplug_context.next_handle++; - /* handle the unlikely case of handle overflow */ - if (hid_hotplug_context.next_handle < 0) - { - hid_hotplug_context.next_handle = 1; - } + /* Reap the monitor thread first in case it is exiting (or has exited) + after the removal of its last callback, releasing the previous + monitoring context with it (may temporarily drop the mutex) */ + hid_internal_hotplug_cleanup(); /* Append a new callback to the end */ if (hid_hotplug_context.hotplug_cbs != NULL) { @@ -1490,37 +1700,56 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven last->next = hotplug_cb; } else { - /* Reap the monitor thread first in case it is exiting (or has exited) - after the removal of its last callback, and release the previous - monitoring context with it */ - hid_internal_hotplug_cleanup(); + int enumerate_failure = 0; + const char *monitor_error = NULL; /* Prepare a UDEV context to run monitoring on */ hid_hotplug_context.udev_ctx = udev_new(); - if (!hid_hotplug_context.udev_ctx) - { - pthread_mutex_unlock(&hid_hotplug_context.mutex); - free(hotplug_cb); - register_global_error("Couldn't create udev context"); - return -1; + if (!hid_hotplug_context.udev_ctx) { + monitor_error = "Couldn't create udev context"; } - hid_hotplug_context.mon = udev_monitor_new_from_netlink(hid_hotplug_context.udev_ctx, "udev"); - if (!hid_hotplug_context.mon) { - udev_unref(hid_hotplug_context.udev_ctx); - hid_hotplug_context.udev_ctx = NULL; + if (!monitor_error) { + hid_hotplug_context.mon = udev_monitor_new_from_netlink(hid_hotplug_context.udev_ctx, "udev"); + if (!hid_hotplug_context.mon) { + monitor_error = "Couldn't create udev monitor"; + } + } + if (!monitor_error && udev_monitor_filter_add_match_subsystem_devtype(hid_hotplug_context.mon, "hidraw", NULL) < 0) { + monitor_error = "Couldn't add the hidraw filter to the udev monitor"; + } + if (!monitor_error && udev_monitor_enable_receiving(hid_hotplug_context.mon) < 0) { + monitor_error = "Couldn't enable receiving on the udev monitor"; + } + if (!monitor_error) { + /* 0 is a valid file descriptor: only negative values are errors */ + hid_hotplug_context.monitor_fd = udev_monitor_get_fd(hid_hotplug_context.mon); + if (hid_hotplug_context.monitor_fd < 0) { + monitor_error = "Couldn't get the udev monitor file descriptor"; + } + } + + if (!monitor_error) { + /* After monitoring is all set up, enumerate all devices: a failure + here would leave pre-connected devices without their "left" + events later, so it fails the registration (unlike an empty + system, which is not an error) */ + hid_hotplug_context.devs = hid_internal_enumerate(0, 0, &enumerate_failure); + if (!enumerate_failure) { + register_global_error(NULL); + } + } + + if (monitor_error || enumerate_failure) { + hid_internal_hotplug_release_monitor(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); - register_global_error("Couldn't create udev monitor"); + if (monitor_error) { + register_global_error(monitor_error); + } + /* on enumerate_failure the global error is already set by the enumeration */ return -1; } - udev_monitor_filter_add_match_subsystem_devtype(hid_hotplug_context.mon, "hidraw", NULL); - udev_monitor_enable_receiving(hid_hotplug_context.mon); - hid_hotplug_context.monitor_fd = udev_monitor_get_fd(hid_hotplug_context.mon); - - /* After monitoring is all set up, enumerate all devices */ - hid_hotplug_context.devs = hid_enumerate(0, 0); - register_global_error(NULL); /* an empty system is not a registration failure */ /* Don't forget to actually register the callback */ hid_hotplug_context.hotplug_cbs = hotplug_cb; @@ -1528,24 +1757,13 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven /* Start the thread that will be doing the event scanning */ if (pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL) != 0) { hid_hotplug_context.hotplug_cbs = NULL; - hid_free_enumeration(hid_hotplug_context.devs); - hid_hotplug_context.devs = NULL; - udev_monitor_unref(hid_hotplug_context.mon); - hid_hotplug_context.mon = NULL; - udev_unref(hid_hotplug_context.udev_ctx); - hid_hotplug_context.udev_ctx = NULL; - hid_hotplug_context.monitor_fd = -1; + hid_internal_hotplug_release_monitor(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); register_global_error("Couldn't create the hotplug monitor thread"); return -1; } - hid_hotplug_context.thread_started = 1; - } - - /* Return allocated handle */ - if (callback_handle != NULL) { - *callback_handle = hotplug_cb->handle; + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_RUNNING; } if ((flags & HID_API_HOTPLUG_ENUMERATE) && (events & HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED)) { @@ -1553,6 +1771,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven delivers it asynchronously as the initial pass of synthetic "arrived" events, before any live events for this callback and never from within this call */ + int snapshot_failure = 0; struct hid_device_info **replay_tail = &hotplug_cb->replay; for (struct hid_device_info *device = hid_hotplug_context.devs; device != NULL; device = device->next) { if (!hid_internal_match_device_id(device->vendor_id, device->product_id, hotplug_cb->vendor_id, hotplug_cb->product_id)) { @@ -1560,17 +1779,38 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven } *replay_tail = hid_internal_copy_device_info(device); if (*replay_tail == NULL) { - /* Out of memory: deliver as much of the snapshot as we could copy */ + snapshot_failure = 1; break; } replay_tail = &(*replay_tail)->next; } - if (hotplug_cb->replay != NULL) { - hid_hotplug_context.replay_pending = 1; + + if (snapshot_failure) { + /* The initial pass is all-or-nothing (each device connection is + reported exactly once - never "neither"): unwind the whole + registration. No event can have been delivered yet: the mutex + was held since the callback was linked in. */ + for (struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; *current != NULL; current = &(*current)->next) { + if (*current == hotplug_cb) { + *current = hotplug_cb->next; + break; + } + } + hid_free_enumeration(hotplug_cb->replay); + /* Reap the monitor thread in case this was the only callback */ + hid_internal_hotplug_cleanup(); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Couldn't allocate the device snapshot for the hotplug enumerate pass"); + return -1; } } - hid_internal_hotplug_cleanup(); + /* Return the allocated handle: written before the mutex is released, i.e. + before any event can be delivered to the callback */ + if (callback_handle != NULL) { + *callback_handle = hotplug_cb->handle; + } pthread_mutex_unlock(&hid_hotplug_context.mutex); @@ -1579,18 +1819,29 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_callback_handle callback_handle) { + unsigned char machinery_ready; + if (callback_handle <= 0) { register_global_error("Invalid hotplug callback handle"); return -1; } - if (!hid_hotplug_context.mutex_ready) { + pthread_mutex_lock(&hid_hotplug_startup_mutex); + machinery_ready = hid_hotplug_context.mutex_ready; + pthread_mutex_unlock(&hid_hotplug_startup_mutex); + + if (!machinery_ready) { register_global_error("No hotplug callbacks are registered"); return -1; } pthread_mutex_lock(&hid_hotplug_context.mutex); + /* Reap a monitor thread whose last callback removed itself on the monitor + thread: without this, the monitoring context would linger until the next + registration or hid_exit (may temporarily drop the mutex) */ + hid_internal_hotplug_cleanup(); + if (hid_hotplug_context.hotplug_cbs == NULL) { pthread_mutex_unlock(&hid_hotplug_context.mutex); register_global_error("No hotplug callbacks are registered"); @@ -1602,6 +1853,11 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_call /* Remove this notification */ for (struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; *current != NULL; current = &(*current)->next) { if ((*current)->handle == callback_handle) { + /* A record deregistered from within a callback (awaiting its + postponed removal) is already gone for the caller: not found */ + if (!(*current)->events) { + break; + } /* Check if we were already in a locked state, as we are NOT allowed to remove any callbacks if we are */ if (hid_hotplug_context.mutex_in_use) { (*current)->events = 0; From b27d1d0a7a3dd9c133d304cef4996f313be04283 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 01:12:08 +0300 Subject: [PATCH 03/10] linux: fix the hotplug teardown races and the monitor-thread error writes - Never destroy the hotplug mutex: hid_exit() could destroy it under a concurrent register/deregister. It is now created once (pthread_once) and lives for the process; teardown is gated by an `exiting` flag set inside the mutex, and hid_exit() loops its cleanup until the callback list is empty and the monitor thread is reaped. - Keep the global error string off HIDAPI's internal monitor thread (an application cannot serialize hid_error(NULL) against it): a quiet flag through create_device_info_for_device()/get_hid_report_descriptor*(), and a register/deregister nested in a callback no longer writes or clears it. - hid_internal_enumerate(): check udev_enumerate_add_match_subsystem() and udev_enumerate_scan_devices(), so a failed scan fails the registration instead of silently caching an empty device list. - Copy the whole arrival up front (dropping it entirely on OOM) instead of dispatching a cache entry with a truncated `next`. - Allocate the callback handle after the cleanup that may drop the mutex, and return it to the counter on every failure path. - The monitor thread now releases its context and detaches itself when its last callback deregistered itself from within a callback. - A dead udev monitor socket (POLLHUP/POLLNVAL, or a persistent POLLERR) tears the monitoring context down instead of dropping events forever. - Match a remove uevent without DEVNAME by syspath, so a stale cache entry cannot swallow the arrival of the next device on the same /dev/hidrawN. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 627 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 484 insertions(+), 143 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 000fa7489..ed4374ebe 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -430,15 +430,19 @@ static int get_next_hid_usage(const __u8 *report_descriptor, __u32 size, struct /* * Retrieves the hidraw report descriptor from a file. * When using this form, /device/report_descriptor, elevated privileges are not required. + * quiet: don't touch the global error string - for the callers on HIDAPI's + * internal monitor thread, which never writes it (see hidapi.h). */ -static int get_hid_report_descriptor(const char *rpt_path, struct hidraw_report_descriptor *rpt_desc) +static int get_hid_report_descriptor(const char *rpt_path, struct hidraw_report_descriptor *rpt_desc, int quiet) { int rpt_handle; ssize_t res; rpt_handle = open(rpt_path, O_RDONLY | O_CLOEXEC); if (rpt_handle < 0) { - register_global_error_format("open failed (%s): %s", rpt_path, strerror(errno)); + if (!quiet) { + register_global_error_format("open failed (%s): %s", rpt_path, strerror(errno)); + } return -1; } @@ -451,7 +455,9 @@ static int get_hid_report_descriptor(const char *rpt_path, struct hidraw_report_ memset(rpt_desc, 0x0, sizeof(*rpt_desc)); res = read(rpt_handle, rpt_desc->value, HID_MAX_DESCRIPTOR_SIZE); if (res < 0) { - register_global_error_format("read failed (%s): %s", rpt_path, strerror(errno)); + if (!quiet) { + register_global_error_format("read failed (%s): %s", rpt_path, strerror(errno)); + } } rpt_desc->size = (__u32) res; @@ -459,8 +465,9 @@ static int get_hid_report_descriptor(const char *rpt_path, struct hidraw_report_ return (int) res; } -/* return size of the descriptor, or -1 on failure */ -static int get_hid_report_descriptor_from_sysfs(const char *sysfs_path, struct hidraw_report_descriptor *rpt_desc) +/* return size of the descriptor, or -1 on failure + (quiet: see get_hid_report_descriptor) */ +static int get_hid_report_descriptor_from_sysfs(const char *sysfs_path, struct hidraw_report_descriptor *rpt_desc, int quiet) { int res = -1; /* Construct /device/report_descriptor */ @@ -470,7 +477,7 @@ static int get_hid_report_descriptor_from_sysfs(const char *sysfs_path, struct h return -1; snprintf(rpt_path, rpt_path_len, "%s/device/report_descriptor", sysfs_path); - res = get_hid_report_descriptor(rpt_path, rpt_desc); + res = get_hid_report_descriptor(rpt_path, rpt_desc, quiet); free(rpt_path); return res; @@ -652,7 +659,9 @@ static int parse_uevent_info(const char *uevent, unsigned *bus_type, } -static struct hid_device_info * create_device_info_for_device(struct udev_device *raw_dev) +/* quiet: don't touch the global error string - for the callers on HIDAPI's + internal monitor thread, which never writes it (see hidapi.h) */ +static struct hid_device_info * create_device_info_for_device(struct udev_device *raw_dev, int quiet) { struct hid_device_info *root = NULL; struct hid_device_info *cur_dev = NULL; @@ -817,7 +826,7 @@ static struct hid_device_info * create_device_info_for_device(struct udev_device /* Usage Page and Usage */ if (sysfs_path) { - result = get_hid_report_descriptor_from_sysfs(sysfs_path, &report_desc); + result = get_hid_report_descriptor_from_sysfs(sysfs_path, &report_desc, quiet); } else { result = -1; @@ -900,7 +909,7 @@ static struct hid_device_info * create_device_info_for_hid_device(hid_device *de /* Open a udev device from the dev_t. 'c' means character device. */ udev_dev = udev_device_new_from_devnum(udev, 'c', s.st_rdev); if (udev_dev) { - root = create_device_info_for_device(udev_dev); + root = create_device_info_for_device(udev_dev, 0); } if (!root) { @@ -932,8 +941,8 @@ enum hid_hotplug_thread_state { HID_HOTPLUG_THREAD_NONE, /* The monitor thread is running */ HID_HOTPLUG_THREAD_RUNNING, - /* The monitor thread has announced its exit and no longer touches any - shared state; it awaits pthread_join */ + /* The monitor thread has released the monitoring context, announced its + exit and no longer touches any shared state; it awaits pthread_join */ HID_HOTPLUG_THREAD_FINISHED }; @@ -955,9 +964,21 @@ static struct hid_hotplug_context { pthread_mutex_t mutex; /* Boolean flags */ + /* The one-time initialization of the mutex succeeded (written once, under + pthread_once); the mutex lives for the rest of the process */ unsigned char mutex_ready; unsigned char mutex_in_use; unsigned char cb_list_dirty; + /* hid_exit() is tearing the hotplug machinery down */ + unsigned char exiting; + /* The udev monitor socket died; no more events (see hotplug_thread) */ + unsigned char monitor_dead; + + /* The number of threads waiting inside hid_internal_hotplug_cleanup() for + the monitor thread to wind down. While it is non-zero, the thread stays + joinable and one of the waiters joins it; with nobody waiting, it + detaches itself instead (see hotplug_thread) */ + unsigned int thread_waiters; /* HIDAPI unique callback handle counter */ hid_hotplug_callback_handle next_handle; @@ -987,12 +1008,12 @@ struct hid_hotplug_callback { struct hid_hotplug_callback *next; }; -static void hid_internal_hotplug_remove_postponed() +static void hid_internal_hotplug_remove_postponed(void) { /* Unregister the callbacks whose removal was postponed */ /* This function is always called inside a locked mutex */ /* However, any actions are only allowed if the mutex is NOT in use and if the DIRTY flag is set */ - if (!hid_hotplug_context.mutex_ready || hid_hotplug_context.mutex_in_use || !hid_hotplug_context.cb_list_dirty) { + if (hid_hotplug_context.mutex_in_use || !hid_hotplug_context.cb_list_dirty) { return; } @@ -1016,7 +1037,8 @@ static void hid_internal_hotplug_remove_postponed() /* Releases a (possibly partially initialized) monitoring context. Called with the mutex held, only when the monitor thread cannot touch it - anymore: before it is started, or after it has been joined. */ + anymore: before it is started, after it has been joined, or by the monitor + thread itself right before it announces its own exit. Idempotent. */ static void hid_internal_hotplug_release_monitor(void) { hid_free_enumeration(hid_hotplug_context.devs); @@ -1036,9 +1058,9 @@ static void hid_internal_hotplug_release_monitor(void) callback is gone. Called with the mutex held exactly once (and not in use); the mutex is temporarily dropped while waiting for/joining the thread, so the caller must re-validate any cached state after this returns. */ -static void hid_internal_hotplug_cleanup() +static void hid_internal_hotplug_cleanup(void) { - if (!hid_hotplug_context.mutex_ready || hid_hotplug_context.mutex_in_use) { + if (hid_hotplug_context.mutex_in_use) { return; } @@ -1072,78 +1094,143 @@ static void hid_internal_hotplug_cleanup() /* HID_HOTPLUG_THREAD_RUNNING: drop the mutex so the thread can acquire it, observe the empty callback list and announce its exit, then - re-evaluate from scratch */ + re-evaluate from scratch. + Announcing the wait keeps the thread joinable: with nobody waiting + for it, it detaches itself (see hotplug_thread). The thread reads + the counter under this mutex, which is held continuously from the + decrement below to the next increment (or to a return, which leaves + the list non-empty or the thread already reaped), so it can never + observe a waiter that is in fact waiting. */ + hid_hotplug_context.thread_waiters++; pthread_mutex_unlock(&hid_hotplug_context.mutex); poll(NULL, 0, 1); pthread_mutex_lock(&hid_hotplug_context.mutex); + hid_hotplug_context.thread_waiters--; } } -/* Protects the first-time initialization of the hotplug machinery (and its - teardown in hid_exit) against concurrent hotplug registrations */ -static pthread_mutex_t hid_hotplug_startup_mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_once_t hid_hotplug_init_once = PTHREAD_ONCE_INIT; -static void hid_internal_hotplug_init() +/* The one-time initialization of the hotplug mutex, run by pthread_once(). + On failure mutex_ready stays 0 and the hotplug API remains unavailable. */ +static void hid_internal_hotplug_init_once(void) { - pthread_mutex_lock(&hid_hotplug_startup_mutex); - if (!hid_hotplug_context.mutex_ready) { - /* Initialize the mutex as recursive */ - pthread_mutexattr_t attr; - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&hid_hotplug_context.mutex, &attr); - pthread_mutexattr_destroy(&attr); + pthread_mutexattr_t attr; - /* Set state to Ready */ - hid_hotplug_context.mutex_in_use = 0; - hid_hotplug_context.cb_list_dirty = 0; - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; - hid_hotplug_context.monitor_fd = -1; - /* The handles are monotonic and not reused while the library remains - initialized (see hidapi.h); the counter survives hid_exit */ - if (hid_hotplug_context.next_handle < FIRST_HOTPLUG_CALLBACK_HANDLE) - hid_hotplug_context.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE; - hid_hotplug_context.mutex_ready = 1; + if (pthread_mutexattr_init(&attr) != 0) { + return; } - pthread_mutex_unlock(&hid_hotplug_startup_mutex); -} + /* The mutex must be recursive: a callback runs with it held and is + allowed to call hid_hotplug_register_callback() / + hid_hotplug_deregister_callback() */ + if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0) { + pthread_mutexattr_destroy(&attr); + return; + } + if (pthread_mutex_init(&hid_hotplug_context.mutex, &attr) != 0) { + pthread_mutexattr_destroy(&attr); + return; + } + pthread_mutexattr_destroy(&attr); -static void hid_internal_hotplug_exit() + hid_hotplug_context.monitor_fd = -1; + /* The handles are monotonic and never reused (see hidapi.h); the counter + survives hid_exit */ + hid_hotplug_context.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE; + + /* Publish the mutex as usable, last */ + hid_hotplug_context.mutex_ready = 1; +} + +/* Ensures the hotplug mutex exists. Returns 0 when the hotplug machinery is + usable, -1 when it could not be initialized (locking an uninitialized mutex + is undefined behavior, so the caller must fail). + The mutex is process-lifetime: it is deliberately never destroyed. + Destroying it in hid_exit() would race the concurrent (and allowed) + hid_hotplug_register_callback()/hid_hotplug_deregister_callback() calls + that are about to lock it - they can only re-check the state AFTER locking, + so the mutex itself must stay valid; teardown is gated by the `exiting` + flag INSIDE the mutex instead. There is deliberately no bootstrap lock + around the initialization either: any lock ordered outside the hotplug + mutex would deadlock against a registration made from within a callback + (which already holds the hotplug mutex); pthread_once() provides both the + one-time guarantee and the memory synchronization for reading mutex_ready. */ +static int hid_internal_hotplug_init(void) { - unsigned char machinery_ready; + pthread_once(&hid_hotplug_init_once, hid_internal_hotplug_init_once); - /* Never hold the startup mutex while waiting for the hotplug mutex: the - monitor thread takes the startup mutex from within callbacks (nested - register/deregister calls) while holding the hotplug mutex */ - pthread_mutex_lock(&hid_hotplug_startup_mutex); - machinery_ready = hid_hotplug_context.mutex_ready; - pthread_mutex_unlock(&hid_hotplug_startup_mutex); + return hid_hotplug_context.mutex_ready ? 0 : -1; +} - if (!machinery_ready) { +static void hid_internal_hotplug_exit(void) +{ + if (hid_internal_hotplug_init() != 0) { + /* The hotplug mutex could not be created: nothing can ever have been + registered, and there is nothing to tear down */ return; } pthread_mutex_lock(&hid_hotplug_context.mutex); - struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; - /* Remove all callbacks from the list, dropping any undelivered snapshots */ - while (*current) { - struct hid_hotplug_callback* next = (*current)->next; - hid_free_enumeration((*current)->replay); - free(*current); - *current = next; + + if (hid_hotplug_context.exiting) { + /* Another hid_exit() is already tearing the machinery down */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + return; } - /* Reap the monitor thread (temporarily dropping the mutex) and release the monitoring context */ - hid_internal_hotplug_cleanup(); - pthread_mutex_unlock(&hid_hotplug_context.mutex); - /* The monitor thread is joined by now and hid_exit is (per the API - thread-safety rules) serialized against every other HIDAPI call except - hid_hotplug_(de)register_callback on other threads - which require the - machinery to be observed ready first */ - pthread_mutex_lock(&hid_hotplug_startup_mutex); - hid_hotplug_context.mutex_ready = 0; - pthread_mutex_destroy(&hid_hotplug_context.mutex); - pthread_mutex_unlock(&hid_hotplug_startup_mutex); + if (hid_hotplug_context.mutex_in_use) { + /* hid_exit() from within a hotplug callback has undefined behavior + (see hidapi.h); degrade gracefully instead of corrupting the + dispatch in flight below this frame: mark every callback for + removal and let the monitor thread wind itself down (releasing the + monitoring context) once the dispatch unwinds */ + for (struct hid_hotplug_callback *callback = hid_hotplug_context.hotplug_cbs; callback; callback = callback->next) { + if (callback->events) { + callback->events = 0; + hid_hotplug_context.cb_list_dirty = 1; + } + } + pthread_mutex_unlock(&hid_hotplug_context.mutex); + return; + } + + /* Close the hotplug API for the duration of the teardown: a concurrent + hid_hotplug_register_callback()/hid_hotplug_deregister_callback() + (allowed by the thread-safety contract) fails/no-ops instead of + re-arming the machinery while it is being torn down - + hid_internal_hotplug_cleanup() below temporarily drops the mutex + while reaping the monitor thread */ + hid_hotplug_context.exiting = 1; + + for (;;) { + struct hid_hotplug_callback **current = &hid_hotplug_context.hotplug_cbs; + /* Remove all callbacks from the list, dropping any undelivered snapshots */ + while (*current) { + struct hid_hotplug_callback* next = (*current)->next; + hid_free_enumeration((*current)->replay); + free(*current); + *current = next; + } + hid_hotplug_context.cb_list_dirty = 0; + + /* Reap the monitor thread (temporarily dropping the mutex) and + release the monitoring context. Repeat until the teardown is + complete: `exiting` keeps anything from re-arming the machinery + while the mutex is dropped, and a concurrent deregistration may + claim the join itself - in which case the thread is observed + already reaped (HID_HOTPLUG_THREAD_NONE) here */ + hid_internal_hotplug_cleanup(); + + if (hid_hotplug_context.hotplug_cbs == NULL + && hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { + break; + } + } + + /* Re-open the hotplug API: the library may be initialized/used again */ + hid_hotplug_context.exiting = 0; + + pthread_mutex_unlock(&hid_hotplug_context.mutex); } /* Serializes the implicit initialization: the hotplug API allows concurrent @@ -1224,8 +1311,24 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, } return NULL; } - udev_enumerate_add_match_subsystem(enumerate, "hidraw"); - udev_enumerate_scan_devices(enumerate); + if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) { + udev_enumerate_unref(enumerate); + udev_unref(udev); + register_global_error("Couldn't add the hidraw subsystem match to the udev enumeration"); + if (failure) { + *failure = 1; + } + return NULL; + } + if (udev_enumerate_scan_devices(enumerate) < 0) { + udev_enumerate_unref(enumerate); + udev_unref(udev); + register_global_error("Couldn't scan the udev devices"); + if (failure) { + *failure = 1; + } + return NULL; + } devices = udev_enumerate_get_list_entry(enumerate); /* For each item, see if it matches the vid/pid, and if so create a udev_device record for it */ @@ -1257,7 +1360,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (!raw_dev) continue; - tmp = create_device_info_for_device(raw_dev); + tmp = create_device_info_for_device(raw_dev, 0); if (tmp) { if (cur_dev) { cur_dev->next = tmp; @@ -1439,6 +1542,8 @@ static struct hid_device_info *hid_internal_find_device_by_path(const char *path static void hid_internal_hotplug_process_arrival(struct hid_device_info *info) { hid_hotplug_callback_handle dispatch_bound; + struct hid_device_info *copies = NULL; + struct hid_device_info **copies_tail = &copies; if (info == NULL) { return; @@ -1453,6 +1558,25 @@ static void hid_internal_hotplug_process_arrival(struct hid_device_info *info) return; } + /* Copy every usage entry up front, for the one-entry-per-invocation + dispatch below (the callback must always see device->next == NULL, and + a copy keeps the cache walkable for the snapshots of callbacks + registered from within a callback). On failure the whole arrival is + dropped BEFORE anything is observable: dispatching entries of the live + cache chain instead would temporarily truncate the cache, and a + callback registered during such a dispatch would miss the truncated + entries in its snapshot - yet receive their "left" events later. */ + for (struct hid_device_info *info_cur = info; info_cur; info_cur = info_cur->next) { + *copies_tail = hid_internal_copy_device_info(info_cur); + if (*copies_tail == NULL) { + /* Out of memory: this device is never reported at all */ + hid_free_enumeration(copies); + hid_free_enumeration(info); + return; + } + copies_tail = &(*copies_tail)->next; + } + /* Append to the cache BEFORE dispatching, so an ENUMERATE snapshot taken by a callback registered from within a callback captures the whole device rather than the in-flight event */ @@ -1469,22 +1593,13 @@ static void hid_internal_hotplug_process_arrival(struct hid_device_info *info) /* Freeze the dispatch to the callbacks registered up to this point */ dispatch_bound = hid_hotplug_context.next_handle - 1; - for (struct hid_device_info *info_cur = info; info_cur; info_cur = info_cur->next) { - /* One usage entry per invocation: the callback must always see - device->next == NULL, and a copy keeps the cache walkable for the - snapshots of callbacks registered from within a callback */ - struct hid_device_info *copy = hid_internal_copy_device_info(info_cur); - if (copy != NULL) { - hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); - hid_free_enumeration(copy); - } else { - /* Out of memory: deliver the cache entry itself, temporarily - detached from the list */ - struct hid_device_info *info_next = info_cur->next; - info_cur->next = NULL; - hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); - info_cur->next = info_next; - } + while (copies != NULL) { + struct hid_device_info *copy = copies; + copies = copy->next; + /* One usage entry per invocation */ + copy->next = NULL; + hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); + hid_free_enumeration(copy); } } @@ -1527,6 +1642,29 @@ static void hid_internal_hotplug_process_removal(const char *devnode) } } +/* Report every cached device as LEFT and clear the cache; used when the + monitoring stops while devices are still connected (a dead monitor socket). + Only ever runs on the monitor thread, with the mutex held. */ +static void hid_internal_hotplug_drop_all_devices(void) +{ + /* Freeze the dispatch to the callbacks registered up to this point + (see hid_internal_hotplug_process_arrival). The whole cache is + detached before dispatching, like in hid_internal_hotplug_process_removal. */ + hid_hotplug_callback_handle dispatch_bound = hid_hotplug_context.next_handle - 1; + struct hid_device_info *removed = hid_hotplug_context.devs; + + hid_hotplug_context.devs = NULL; + + while (removed != NULL) { + struct hid_device_info *info = removed; + removed = info->next; + /* One usage entry per invocation: the callback always sees next == NULL */ + info->next = NULL; + hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); + hid_free_enumeration(info); + } +} + /* Dispatch one udev monitor event. Only ever runs on the monitor thread, with the mutex held. */ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) @@ -1537,21 +1675,51 @@ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) } if (!strcmp(action, "add")) { - /* We create a list of all usages on this UDEV device */ - hid_internal_hotplug_process_arrival(create_device_info_for_device(raw_dev)); + /* We create a list of all usages on this UDEV device. + A device whose information cannot be read/parsed stays out of the + cache and is invisible to the callbacks - exactly as it would be + invisible to hid_enumerate(); there is nothing meaningful to + deliver instead. quiet: this runs on the monitor thread, which + never writes the global error string (see hidapi.h). */ + hid_internal_hotplug_process_arrival(create_device_info_for_device(raw_dev, 1)); } else if (!strcmp(action, "remove")) { const char *devnode = udev_device_get_devnode(raw_dev); + char devnode_buf[32]; + if (devnode == NULL) { + /* A remove event does not always carry the device node. The + cache is keyed by the node path, which for a hidraw device is + always "/dev/": reconstruct it, or the stale cache + entry would suppress - as a duplicate - the arrival of the + next device that reuses the same node */ + const char *sysname = udev_device_get_sysname(raw_dev); + if (sysname != NULL) { + int len = snprintf(devnode_buf, sizeof(devnode_buf), "/dev/%s", sysname); + if (len > 0 && (size_t)len < sizeof(devnode_buf)) { + devnode = devnode_buf; + } + } + } if (devnode != NULL) { hid_internal_hotplug_process_removal(devnode); } } } +/* Consecutive poll() reports of an error condition on the udev monitor + socket - with a drain attempt in between each - after which the socket is + considered irrecoverably dead. Netlink receive-buffer overruns (ENOBUFS) + raise POLLERR but clear once the pending error is consumed by a receive + attempt, so a bounded number of retries filters those out. */ +#define HID_HOTPLUG_SOCKET_ERROR_POLL_LIMIT 100 + static void* hotplug_thread(void* user_data) { /* The monitor file descriptor is immutable while this thread runs: it is - set up before the thread starts and released only after it is joined */ + set up before the thread starts and released only after it is joined + (or by this thread itself, once it stops polling the descriptor) */ const int monitor_fd = hid_hotplug_context.monitor_fd; + int socket_error_polls = 0; + int socket_dead = 0; (void) user_data; @@ -1575,24 +1743,64 @@ static void* hotplug_thread(void* user_data) } if (hid_hotplug_context.hotplug_cbs == NULL) { - /* The last callback is gone: announce the exit under the mutex, - then stop touching any shared state */ - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_FINISHED; + /* The last callback is gone: release the monitoring context (the + device cache, the udev monitor and its context) right away - + when the last callback removed itself from within a callback, + no further hotplug call is guaranteed to come and reap it - + then announce the exit under the mutex and stop touching any + shared state. A monitoring context created after this point + belongs to a new thread; the release is idempotent, so a + claimant repeating it is a no-op. */ + hid_internal_hotplug_release_monitor(); + hid_hotplug_context.monitor_dead = 0; + if (hid_hotplug_context.thread_waiters > 0) { + /* A hotplug call is waiting inside + hid_internal_hotplug_cleanup() to reap this thread: stay + joinable and let it claim the join */ + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_FINISHED; + } + else { + /* Nobody is waiting for this thread, and nobody ever may: the + last callback can have deregistered itself from within a + callback - i.e. on this very thread - after which the + application is not required to make any further hotplug + call. Release the thread instead of leaving it unjoined + forever: nothing is left to reap */ + pthread_detach(pthread_self()); + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + } stop = 1; } else { + if (socket_dead && !hid_hotplug_context.monitor_dead) { + /* The udev monitor socket is dead: no hotplug event can ever + be observed again. Fail into a well-defined state instead + of silently dropping events from now on: report every + cached device as removed (keeping the arrived/left events + paired for every callback), release the monitor, and mark + the machinery dead - new registrations are refused until + it winds down completely. The global error string is not + touched: HIDAPI's internal threads never write it (see + hidapi.h). */ + hid_hotplug_context.monitor_dead = 1; + hid_internal_hotplug_drop_all_devices(); + hid_internal_hotplug_release_monitor(); + } + /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE before any queued live events */ hid_internal_hotplug_process_replays(); - /* Drain and dispatch the events queued on the (non-blocking) - udev monitor socket */ - for (;;) { - struct udev_device *raw_dev = udev_monitor_receive_device(hid_hotplug_context.mon); - if (raw_dev == NULL) { - break; + if (!socket_dead) { + /* Drain and dispatch the events queued on the (non-blocking) + udev monitor socket */ + for (;;) { + struct udev_device *raw_dev = udev_monitor_receive_device(hid_hotplug_context.mon); + if (raw_dev == NULL) { + break; + } + hid_internal_hotplug_process_event(raw_dev); + udev_device_unref(raw_dev); } - hid_internal_hotplug_process_event(raw_dev); - udev_device_unref(raw_dev); } } @@ -1602,6 +1810,14 @@ static void* hotplug_thread(void* user_data) break; } + if (socket_dead) { + /* No monitor socket to wait on anymore (it has been released + above): keep pacing the loop for the replay flushes and the + exit check */ + poll(NULL, 0, 5); + continue; + } + /* Wait for udev events; the timeout paces the mutex retries and caps the latency of the initial HID_API_HOTPLUG_ENUMERATE passes. 5 msec seems reasonable; don't set too low to avoid high CPU usage. */ @@ -1610,54 +1826,109 @@ static void* hotplug_thread(void* user_data) fds.revents = 0; ret = poll(&fds, 1, 5); if (ret > 0 && !(fds.revents & POLLIN)) { - /* Monitor socket failure: without this, a dead socket would make - poll() return immediately and busy-spin this loop; keep the - 5 msec pace instead (events are lost either way) */ - poll(NULL, 0, 5); + /* An error condition with no data to read */ + if (fds.revents & (POLLHUP | POLLNVAL)) { + /* Unambiguously dead: the socket got closed or invalidated */ + socket_dead = 1; + } else if (++socket_error_polls >= HID_HOTPLUG_SOCKET_ERROR_POLL_LIMIT) { + /* Persistent POLLERR: an error condition that survives this + many drain attempts is not a recoverable overrun */ + socket_dead = 1; + } + if (!socket_dead) { + /* Without this, the error condition would make poll() return + immediately and busy-spin this loop; keep the 5 msec pace */ + poll(NULL, 0, 5); + } + } else { + socket_error_polls = 0; } } - /* The device cache, the udev monitor and its context stay allocated until - hid_internal_hotplug_cleanup() joins this thread (on a later hotplug - registration/deregistration or at hid_exit): releasing them here would - race a re-created monitoring context */ - return NULL; } int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short vendor_id, unsigned short product_id, int events, int flags, hid_hotplug_callback_fn callback, void *user_data, hid_hotplug_callback_handle *callback_handle) { struct hid_hotplug_callback* hotplug_cb; + int quiet; /* No events can be delivered before the out parameter is written */ if (callback_handle != NULL) { *callback_handle = 0; } + /* Ensure we are ready to actually use the mutex (the mutex is + process-lifetime and never destroyed - see hid_internal_hotplug_init). + This can only fail before any callback was ever registered - i.e. never + on the monitor thread - so the error string is safe to write here */ + if (hid_internal_hotplug_init() != 0) { + register_global_error("Couldn't initialize the hotplug mutex"); + return -1; + } + + /* Lock the mutex to avoid race conditions */ + pthread_mutex_lock(&hid_hotplug_context.mutex); + + /* A registration made from within a callback runs on HIDAPI's internal + monitor thread, which holds this (recursive) mutex for the whole + dispatch - hence mutex_in_use. Such a call must leave the global error + string alone: HIDAPI's internal threads never write it (see hidapi.h), + or an application that reads it with hid_error(NULL) on another thread + would race a write it cannot serialize against. This is why even the + parameter checks below run under the mutex. */ + quiet = hid_hotplug_context.mutex_in_use; + /* Check params */ if (callback == NULL) { - register_global_error("Hotplug callback function is NULL"); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("Hotplug callback function is NULL"); + } return -1; } if (events == 0 || (events & ~(HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED | HID_API_HOTPLUG_EVENT_DEVICE_LEFT))) { - register_global_error("Hotplug events mask contains no valid events or unknown bits"); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("Hotplug events mask contains no valid events or unknown bits"); + } return -1; } if (flags & ~(HID_API_HOTPLUG_ENUMERATE)) { - register_global_error("Hotplug flags mask contains unknown bits"); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("Hotplug flags mask contains unknown bits"); + } return -1; } - /* Implicit hid_init: unlike the other API entry points, concurrent - registrations are allowed - hid_init() serializes itself */ - hid_init(); - /* register_global_error: global error is reset by hid_init */ + if (hid_hotplug_context.exiting) { + /* hid_exit() is tearing the machinery down: it deregisters every + callback itself, so there is nothing to register into */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("hid_exit() is in progress"); + } + return -1; + } + + if (!quiet) { + /* Implicit hid_init: unlike the other API entry points, concurrent + registrations are allowed - hid_init() serializes itself. + A nested registration skips it: the library is initialized by then, + and hid_init() resets the global error string (see above) */ + hid_init(); + /* register_global_error: global error is reset by hid_init */ + } hotplug_cb = (struct hid_hotplug_callback*)calloc(1, sizeof(struct hid_hotplug_callback)); if (hotplug_cb == NULL) { - register_global_error("Failed to allocate a hotplug callback record"); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("Failed to allocate a hotplug callback record"); + } return -1; } @@ -1670,27 +1941,55 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hotplug_cb->callback = callback; hotplug_cb->replay = NULL; - /* Ensure we are ready to actually use the mutex */ - hid_internal_hotplug_init(); + /* Reap the monitor thread first in case it is exiting (or has exited) + after the removal of its last callback, releasing the previous + monitoring context with it (may temporarily drop the mutex) */ + hid_internal_hotplug_cleanup(); + + /* hid_exit() may have started while the mutex was dropped above: fail + instead of re-arming the machinery it is tearing down */ + if (hid_hotplug_context.exiting) { + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + if (!quiet) { + register_global_error("hid_exit() is in progress"); + } + return -1; + } - /* Lock the mutex to avoid race conditions */ - pthread_mutex_lock(&hid_hotplug_context.mutex); + if (hid_hotplug_context.monitor_dead) { + /* The udev monitor socket died (see hotplug_thread): this machinery + can deliver no further events, so refuse to attach to it. It winds + down once the surviving callbacks are deregistered; a registration + after that rebuilds it. */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + if (!quiet) { + register_global_error("The hotplug monitor is not operational (the udev monitor socket failed)"); + } + return -1; + } /* The handles are monotonic and never reused: fail instead of overflowing */ if (hid_hotplug_context.next_handle == INT_MAX) { pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); - register_global_error("Hotplug callback handles exhausted"); + if (!quiet) { + register_global_error("Hotplug callback handles exhausted"); + } return -1; } + /* Allocate the handle only after hid_internal_hotplug_cleanup() above: + it may temporarily drop the mutex, and the dispatch bounds rely on the + callback list being in (monotonic) handle order - a handle allocated + before the drop could get linked in after a younger one. The mutex is + then held continuously from this allocation up to (at least) the + unwinding point of every failure path below, so those paths can safely + return the handle to the counter (a failed registration must not + consume handles). */ hotplug_cb->handle = hid_hotplug_context.next_handle++; - /* Reap the monitor thread first in case it is exiting (or has exited) - after the removal of its last callback, releasing the previous - monitoring context with it (may temporarily drop the mutex) */ - hid_internal_hotplug_cleanup(); - /* Append a new callback to the end */ if (hid_hotplug_context.hotplug_cbs != NULL) { struct hid_hotplug_callback *last = hid_hotplug_context.hotplug_cbs; @@ -1703,6 +2002,13 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven int enumerate_failure = 0; const char *monitor_error = NULL; + /* This branch is never taken on the monitor thread: the callback list + is empty here, while a dispatch always has at least the callback it + is dispatching to in the list (a deregistration from within a + callback only tombstones the record). The global error writes below + - including the hid_internal_enumerate() ones - are therefore always + made by an application thread */ + /* Prepare a UDEV context to run monitoring on */ hid_hotplug_context.udev_ctx = udev_new(); if (!hid_hotplug_context.udev_ctx) { @@ -1741,6 +2047,8 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven } if (monitor_error || enumerate_failure) { + /* The handle never became visible: return it to the counter */ + hid_hotplug_context.next_handle--; hid_internal_hotplug_release_monitor(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); @@ -1757,6 +2065,8 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven /* Start the thread that will be doing the event scanning */ if (pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL) != 0) { hid_hotplug_context.hotplug_cbs = NULL; + /* The handle never became visible: return it to the counter */ + hid_hotplug_context.next_handle--; hid_internal_hotplug_release_monitor(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); @@ -1797,11 +2107,15 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven } } hid_free_enumeration(hotplug_cb->replay); + /* The handle never became visible: return it to the counter */ + hid_hotplug_context.next_handle--; /* Reap the monitor thread in case this was the only callback */ hid_internal_hotplug_cleanup(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); - register_global_error("Couldn't allocate the device snapshot for the hotplug enumerate pass"); + if (!quiet) { + register_global_error("Couldn't allocate the device snapshot for the hotplug enumerate pass"); + } return -1; } } @@ -1819,32 +2133,59 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_callback_handle callback_handle) { - unsigned char machinery_ready; + int quiet; - if (callback_handle <= 0) { - register_global_error("Invalid hotplug callback handle"); + if (hid_internal_hotplug_init() != 0) { + /* The hotplug mutex could not be created: nothing can ever have been + registered (and this can never run on the monitor thread, so the + error string is safe to write here) */ + register_global_error("No hotplug callbacks are registered"); return -1; } - pthread_mutex_lock(&hid_hotplug_startup_mutex); - machinery_ready = hid_hotplug_context.mutex_ready; - pthread_mutex_unlock(&hid_hotplug_startup_mutex); + pthread_mutex_lock(&hid_hotplug_context.mutex); - if (!machinery_ready) { - register_global_error("No hotplug callbacks are registered"); + /* A deregistration made from within a callback runs on HIDAPI's internal + monitor thread, which never writes the global error string - the + parameter check included (see hid_hotplug_register_callback) */ + quiet = hid_hotplug_context.mutex_in_use; + + if (callback_handle <= 0) { + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("Invalid hotplug callback handle"); + } return -1; } - pthread_mutex_lock(&hid_hotplug_context.mutex); + if (hid_hotplug_context.exiting) { + /* hid_exit() is tearing the machinery down: it deregisters every + callback and invalidates every handle itself */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("No hotplug callbacks are registered"); + } + return -1; + } - /* Reap a monitor thread whose last callback removed itself on the monitor - thread: without this, the monitoring context would linger until the next - registration or hid_exit (may temporarily drop the mutex) */ + /* Reap a monitor thread that has wound down after the removal of its last + callback but has not been joined yet (may temporarily drop the mutex) */ hid_internal_hotplug_cleanup(); + /* hid_exit() may have started while the mutex was dropped above */ + if (hid_hotplug_context.exiting) { + pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (!quiet) { + register_global_error("No hotplug callbacks are registered"); + } + return -1; + } + if (hid_hotplug_context.hotplug_cbs == NULL) { pthread_mutex_unlock(&hid_hotplug_context.mutex); - register_global_error("No hotplug callbacks are registered"); + if (!quiet) { + register_global_error("No hotplug callbacks are registered"); + } return -1; } @@ -1878,7 +2219,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_deregister_callback(hid_hotplug_call pthread_mutex_unlock(&hid_hotplug_context.mutex); - if (result < 0) { + if (result < 0 && !quiet) { register_global_error("Hotplug callback handle not found"); } From 7494623a1d61d37453c6d137239280e3d11cc66c Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 01:26:10 +0300 Subject: [PATCH 04/10] linux: detach the hotplug monitor thread and lock its monitor_fd read TSan (verified live with a racy probe) reported a data race on the read of hid_hotplug_context.monitor_fd in the monitor thread's prologue: it is unsynchronized against the setup of a later monitoring context. Read it under the mutex. Reap the monitor thread by detaching it at creation instead of joining it: when the last callback deregisters itself from within a callback, nothing is guaranteed to ever call into HIDAPI again to join the thread. The thread releases the monitoring context and announces its exit under the mutex, so nothing it owns or touches outlives hid_exit(); pthread_join(), the FINISHED state and the join bookkeeping all go away with it. (pthread_detach(pthread_self()) in the wind-down was the obvious alternative, but it makes ThreadSanitizer abort in its own interceptor - CHECK failed at tsan_rtl_thread.cpp:330, tid == kInvalidTid - which would break TSan runs for everyone.) Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 111 +++++++++++++++++++++------------------------------- 1 file changed, 45 insertions(+), 66 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index ed4374ebe..5c7f020da 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -940,10 +940,7 @@ enum hid_hotplug_thread_state { /* No monitor thread, nothing to join */ HID_HOTPLUG_THREAD_NONE, /* The monitor thread is running */ - HID_HOTPLUG_THREAD_RUNNING, - /* The monitor thread has released the monitoring context, announced its - exit and no longer touches any shared state; it awaits pthread_join */ - HID_HOTPLUG_THREAD_FINISHED + HID_HOTPLUG_THREAD_RUNNING }; static struct hid_hotplug_context { @@ -974,12 +971,6 @@ static struct hid_hotplug_context { /* The udev monitor socket died; no more events (see hotplug_thread) */ unsigned char monitor_dead; - /* The number of threads waiting inside hid_internal_hotplug_cleanup() for - the monitor thread to wind down. While it is non-zero, the thread stays - joinable and one of the waiters joins it; with nobody waiting, it - detaches itself instead (see hotplug_thread) */ - unsigned int thread_waiters; - /* HIDAPI unique callback handle counter */ hid_hotplug_callback_handle next_handle; @@ -1054,10 +1045,11 @@ static void hid_internal_hotplug_release_monitor(void) hid_hotplug_context.monitor_fd = -1; } -/* Reaps the monitor thread and releases the monitoring context once the last - callback is gone. Called with the mutex held exactly once (and not in use); - the mutex is temporarily dropped while waiting for/joining the thread, so - the caller must re-validate any cached state after this returns. */ +/* Waits for the monitor thread to wind down once the last callback is gone (it + releases the monitoring context itself: it is detached, and nothing can + join it - see hotplug_thread). Called with the mutex held exactly once (and + not in use); the mutex is temporarily dropped while waiting for the thread, + so the caller must re-validate any cached state after this returns. */ static void hid_internal_hotplug_cleanup(void) { if (hid_hotplug_context.mutex_in_use) { @@ -1065,47 +1057,23 @@ static void hid_internal_hotplug_cleanup(void) } for (;;) { - pthread_t thread; - /* Before checking if the list is empty, clear any entries whose removal was postponed first */ hid_internal_hotplug_remove_postponed(); if (hid_hotplug_context.hotplug_cbs != NULL || hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { - /* Still serving callbacks, no thread to reap, or a concurrent - caller reaped it already (and possibly started a new context) */ - return; - } - - if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { - /* Claim the join, so no concurrent caller joins the thread twice */ - thread = hid_hotplug_context.thread; - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; - - /* The thread no longer touches the shared state once it has - announced itself FINISHED (under this mutex) */ - hid_internal_hotplug_release_monitor(); - - /* Never block in pthread_join while holding the mutex */ - pthread_mutex_unlock(&hid_hotplug_context.mutex); - pthread_join(thread, NULL); - pthread_mutex_lock(&hid_hotplug_context.mutex); + /* Still serving callbacks, or no thread is running - either it + never started, or it has released the monitoring context and + announced its exit (and a concurrent caller may already have + started a new one) */ return; } /* HID_HOTPLUG_THREAD_RUNNING: drop the mutex so the thread can acquire - it, observe the empty callback list and announce its exit, then - re-evaluate from scratch. - Announcing the wait keeps the thread joinable: with nobody waiting - for it, it detaches itself (see hotplug_thread). The thread reads - the counter under this mutex, which is held continuously from the - decrement below to the next increment (or to a return, which leaves - the list non-empty or the thread already reaped), so it can never - observe a waiter that is in fact waiting. */ - hid_hotplug_context.thread_waiters++; + it, observe the empty callback list, release the monitoring context + and announce its exit; then re-evaluate from scratch */ pthread_mutex_unlock(&hid_hotplug_context.mutex); poll(NULL, 0, 1); pthread_mutex_lock(&hid_hotplug_context.mutex); - hid_hotplug_context.thread_waiters--; } } @@ -1714,15 +1682,21 @@ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) static void* hotplug_thread(void* user_data) { - /* The monitor file descriptor is immutable while this thread runs: it is - set up before the thread starts and released only after it is joined - (or by this thread itself, once it stops polling the descriptor) */ - const int monitor_fd = hid_hotplug_context.monitor_fd; + int monitor_fd; int socket_error_polls = 0; int socket_dead = 0; (void) user_data; + /* The monitor file descriptor is immutable while this thread runs: it is + set up before the thread is started and released either by this thread + itself or after it announced its exit. It is read under the mutex all + the same: an unsynchronized read here would race the setup of the next + monitoring context (whose registration this thread cannot have seen). */ + pthread_mutex_lock(&hid_hotplug_context.mutex); + monitor_fd = hid_hotplug_context.monitor_fd; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + /* This thread takes the mutex with trylock only: it must never block on the mutex, so that it always makes progress towards its exit check while hid_internal_hotplug_cleanup() waits, without holding the mutex, @@ -1753,22 +1727,10 @@ static void* hotplug_thread(void* user_data) claimant repeating it is a no-op. */ hid_internal_hotplug_release_monitor(); hid_hotplug_context.monitor_dead = 0; - if (hid_hotplug_context.thread_waiters > 0) { - /* A hotplug call is waiting inside - hid_internal_hotplug_cleanup() to reap this thread: stay - joinable and let it claim the join */ - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_FINISHED; - } - else { - /* Nobody is waiting for this thread, and nobody ever may: the - last callback can have deregistered itself from within a - callback - i.e. on this very thread - after which the - application is not required to make any further hotplug - call. Release the thread instead of leaving it unjoined - forever: nothing is left to reap */ - pthread_detach(pthread_self()); - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; - } + /* This thread is detached: nothing is left to reap once it + announces its exit (it touches no shared state below this + point, and everything it owned has just been released) */ + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; stop = 1; } else { if (socket_dead && !hid_hotplug_context.monitor_dead) { @@ -2062,8 +2024,25 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven /* Don't forget to actually register the callback */ hid_hotplug_context.hotplug_cbs = hotplug_cb; - /* Start the thread that will be doing the event scanning */ - if (pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL) != 0) { + /* Start the thread that will be doing the event scanning. + The thread is detached: when the last callback deregisters itself + from within a callback, the thread winds down on its own and no + further hotplug call - which is what would join it - is guaranteed + to ever come. It releases the monitoring context and announces + HID_HOTPLUG_THREAD_NONE under the mutex before it returns, so + nothing it owns or touches can outlive hid_exit(). */ + pthread_attr_t attr; + int thread_error = 1; + + if (pthread_attr_init(&attr) == 0) { + if (pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED) == 0 + && pthread_create(&hid_hotplug_context.thread, &attr, &hotplug_thread, NULL) == 0) { + thread_error = 0; + } + pthread_attr_destroy(&attr); + } + + if (thread_error) { hid_hotplug_context.hotplug_cbs = NULL; /* The handle never became visible: return it to the counter */ hid_hotplug_context.next_handle--; From 77d412463464edda6fc547205490a730fb0193ec Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 01:59:09 +0300 Subject: [PATCH 05/10] linux: join the hotplug monitor thread and stop fabricating removals Restore a joinable monitor-thread lifecycle (RUNNING -> FINISHED -> pthread_join -> NONE) so that once hid_exit() returns the thread is provably gone. The detached thread could publish its exit and let hid_exit() return while still running its own epilogue inside the library's text - a crash if the application then dlclose()s. The thread that self-terminates (its last callback deregistered from within a callback) leaves a joinable pthread_t reaped by the next register/deregister/hid_exit, mirroring the macOS deferred join. On a dead udev monitor socket, no longer synthesize DEVICE_LEFT for the cached (still-connected) devices. Instead rebuild the monitor a bounded number of times and, on success, re-enumerate and dispatch only the real deltas that occurred while the transport was down; if recreation ultimately fails, stop monitoring without any fabricated removal, leave the cache intact, and mark the machinery dead so new registrations are refused (the observable failure). The re-enumeration is quiet, so the monitor thread still never writes the global error string. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 333 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 261 insertions(+), 72 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 5c7f020da..fe6817233 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -937,10 +937,20 @@ HID_API_EXPORT const char* HID_API_CALL hid_version_str(void) /* Lifecycle of the udev monitor thread; every transition happens under the hotplug mutex */ enum hid_hotplug_thread_state { - /* No monitor thread, nothing to join */ + /* No monitor thread exists; nothing to join */ HID_HOTPLUG_THREAD_NONE, /* The monitor thread is running */ - HID_HOTPLUG_THREAD_RUNNING + HID_HOTPLUG_THREAD_RUNNING, + /* The monitor thread has returned - it published this state as its last + write under the mutex, immediately before unlocking and returning - but + its pthread_t has not been joined yet. The thread is JOINABLE, not + detached: hid_exit() (and the next register/deregister) join it, so once + hid_exit() returns no monitor-thread instruction can still be executing + inside the library. A detached thread could not guarantee that - after it + published its exit and hid_exit() returned, it could still be running its + own epilogue (unlock/return) in the library's text when the application + calls dlclose(), i.e. resume in unmapped code. */ + HID_HOTPLUG_THREAD_FINISHED }; static struct hid_hotplug_context { @@ -1045,11 +1055,38 @@ static void hid_internal_hotplug_release_monitor(void) hid_hotplug_context.monitor_fd = -1; } -/* Waits for the monitor thread to wind down once the last callback is gone (it - releases the monitoring context itself: it is detached, and nothing can - join it - see hotplug_thread). Called with the mutex held exactly once (and - not in use); the mutex is temporarily dropped while waiting for the thread, - so the caller must re-validate any cached state after this returns. */ +/* Reaps a monitor thread that has finished (published HID_HOTPLUG_THREAD_FINISHED + as its last write under the mutex) but has not been joined yet, turning its + state to HID_HOTPLUG_THREAD_NONE. Called with the mutex held. + + The join is performed WITH the mutex held: a finished thread needs nothing + further from the mutex (it released the monitoring context and published + FINISHED before unlocking, then only runs unlock/return), so joining under + the mutex cannot deadlock and naturally serializes concurrent reapers - + whoever loses the race for the mutex observes HID_HOTPLUG_THREAD_NONE and + returns, so the pthread_t is joined exactly once. + + Never runs on the monitor thread itself: the thread becomes FINISHED only + after it has stopped dispatching, and the callers below are gated by + mutex_in_use (a dispatch in flight), so a thread can never be its own + joiner. */ +static void hid_internal_hotplug_join_thread(void) +{ + if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { + return; + } + + pthread_join(hid_hotplug_context.thread, NULL); + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; +} + +/* Winds the monitor thread down once the last callback is gone and joins it, so + that no monitor-thread code is still executing once this returns. The thread + releases the monitoring context itself, then publishes FINISHED; this joins + the FINISHED thread. Called with the mutex held exactly once (and not in + use); the mutex is temporarily dropped while waiting for a still-RUNNING + thread to notice the empty list, so the caller must re-validate any cached + state after this returns. */ static void hid_internal_hotplug_cleanup(void) { if (hid_hotplug_context.mutex_in_use) { @@ -1060,17 +1097,26 @@ static void hid_internal_hotplug_cleanup(void) /* Before checking if the list is empty, clear any entries whose removal was postponed first */ hid_internal_hotplug_remove_postponed(); - if (hid_hotplug_context.hotplug_cbs != NULL || hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { - /* Still serving callbacks, or no thread is running - either it - never started, or it has released the monitoring context and - announced its exit (and a concurrent caller may already have - started a new one) */ + if (hid_hotplug_context.hotplug_cbs != NULL) { + /* Still serving callbacks - the thread must keep running */ + return; + } + + if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { + /* No thread to join: it never started, or a concurrent caller + already reaped it (and may have started a new one) */ + return; + } + + if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { + /* The thread released the monitoring context and returned: join it */ + hid_internal_hotplug_join_thread(); return; } /* HID_HOTPLUG_THREAD_RUNNING: drop the mutex so the thread can acquire it, observe the empty callback list, release the monitoring context - and announce its exit; then re-evaluate from scratch */ + and publish FINISHED; then re-evaluate and join from scratch */ pthread_mutex_unlock(&hid_hotplug_context.mutex); poll(NULL, 0, 1); pthread_mutex_lock(&hid_hotplug_context.mutex); @@ -1243,7 +1289,12 @@ static int hid_internal_match_device_id(unsigned short vendor_id, unsigned short /* Same as hid_enumerate, but distinguishes a genuine failure from an empty system: *failure (when non-NULL) is set to 1 only when the enumeration itself failed (with the global error set accordingly). */ -static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, unsigned short product_id, int *failure) +/* quiet: don't touch the global error string - for the caller on HIDAPI's + internal monitor thread (the hotplug re-enumeration after a monitor rebuild), + which never writes it (see hidapi.h). A quiet caller must already have + initialized the library, so hid_init() is skipped too (it resets the global + error). An empty result is not a failure in either mode. */ +static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, unsigned short product_id, int quiet, int *failure) { struct udev *udev; struct udev_enumerate *enumerate; @@ -1256,13 +1307,17 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, *failure = 0; } - hid_init(); - /* register_global_error: global error is reset by hid_init */ + if (!quiet) { + hid_init(); + /* register_global_error: global error is reset by hid_init */ + } /* Create the udev object */ udev = udev_new(); if (!udev) { - register_global_error("Couldn't create udev context"); + if (!quiet) { + register_global_error("Couldn't create udev context"); + } if (failure) { *failure = 1; } @@ -1273,7 +1328,9 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, enumerate = udev_enumerate_new(udev); if (!enumerate) { udev_unref(udev); - register_global_error("Couldn't create udev enumeration"); + if (!quiet) { + register_global_error("Couldn't create udev enumeration"); + } if (failure) { *failure = 1; } @@ -1282,7 +1339,9 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) { udev_enumerate_unref(enumerate); udev_unref(udev); - register_global_error("Couldn't add the hidraw subsystem match to the udev enumeration"); + if (!quiet) { + register_global_error("Couldn't add the hidraw subsystem match to the udev enumeration"); + } if (failure) { *failure = 1; } @@ -1291,7 +1350,9 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (udev_enumerate_scan_devices(enumerate) < 0) { udev_enumerate_unref(enumerate); udev_unref(udev); - register_global_error("Couldn't scan the udev devices"); + if (!quiet) { + register_global_error("Couldn't scan the udev devices"); + } if (failure) { *failure = 1; } @@ -1328,7 +1389,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (!raw_dev) continue; - tmp = create_device_info_for_device(raw_dev, 0); + tmp = create_device_info_for_device(raw_dev, quiet); if (tmp) { if (cur_dev) { cur_dev->next = tmp; @@ -1350,7 +1411,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, udev_enumerate_unref(enumerate); udev_unref(udev); - if (root == NULL) { + if (root == NULL && !quiet) { if (vendor_id == 0 && product_id == 0) { register_global_error("No HID devices found in the system."); } else { @@ -1363,7 +1424,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, unsigned short product_id) { - return hid_internal_enumerate(vendor_id, product_id, NULL); + return hid_internal_enumerate(vendor_id, product_id, 0, NULL); } void HID_API_EXPORT hid_free_enumeration(struct hid_device_info *devs) @@ -1491,12 +1552,12 @@ static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotp pthread_mutex_unlock(&hid_hotplug_context.mutex); } -static struct hid_device_info *hid_internal_find_device_by_path(const char *path) +static struct hid_device_info *hid_internal_find_device_in_list(struct hid_device_info *list, const char *path) { if (path == NULL) { return NULL; } - for (struct hid_device_info *device = hid_hotplug_context.devs; device; device = device->next) { + for (struct hid_device_info *device = list; device; device = device->next) { if (device->path && !strcmp(device->path, path)) { return device; } @@ -1504,6 +1565,11 @@ static struct hid_device_info *hid_internal_find_device_by_path(const char *path return NULL; } +static struct hid_device_info *hid_internal_find_device_by_path(const char *path) +{ + return hid_internal_find_device_in_list(hid_hotplug_context.devs, path); +} + /* Handle a device arrival: update the connected-device cache and dispatch the callbacks. Takes ownership of the whole device chain (one entry per usage). Only ever runs on the monitor thread, with the mutex held. */ @@ -1610,29 +1676,122 @@ static void hid_internal_hotplug_process_removal(const char *devnode) } } -/* Report every cached device as LEFT and clear the cache; used when the - monitoring stops while devices are still connected (a dead monitor socket). - Only ever runs on the monitor thread, with the mutex held. */ -static void hid_internal_hotplug_drop_all_devices(void) +/* Reconcile the device cache with a fresh enumeration after the udev monitor + was rebuilt, delivering ONLY the real deltas that happened while the transport + was down: DEVICE_ARRIVED for a device present now but not cached, DEVICE_LEFT + for a cached device genuinely absent now. A device present in both is left + untouched - no event is fabricated for a device that never went anywhere. + The fresh enumeration is adopted as the new cache. Takes ownership of `fresh` + and of the previous cache. Only ever runs on the monitor thread, with the + mutex held. */ +static void hid_internal_hotplug_reconcile(struct hid_device_info *fresh) { /* Freeze the dispatch to the callbacks registered up to this point - (see hid_internal_hotplug_process_arrival). The whole cache is - detached before dispatching, like in hid_internal_hotplug_process_removal. */ + (see hid_internal_hotplug_process_arrival). */ hid_hotplug_callback_handle dispatch_bound = hid_hotplug_context.next_handle - 1; - struct hid_device_info *removed = hid_hotplug_context.devs; - - hid_hotplug_context.devs = NULL; + struct hid_device_info *old = hid_hotplug_context.devs; + + /* Adopt the fresh enumeration as the cache up front: a callback registered + from within one of the dispatches below (handle beyond dispatch_bound, so + excluded from them) then captures the reconciled set in its ENUMERATE + snapshot instead - keeping every arrival/departure exactly-once. */ + hid_hotplug_context.devs = fresh; + + /* ARRIVED for every fresh usage entry whose device was not cached. Membership + is tested against the still-intact `old` cache. A COPY is dispatched (one + entry, next == NULL) so the adopted cache chain stays walkable. */ + for (struct hid_device_info *device = fresh; device != NULL; device = device->next) { + struct hid_device_info *copy; + if (hid_internal_find_device_in_list(old, device->path) != NULL) { + continue; + } + copy = hid_internal_copy_device_info(device); + if (copy == NULL) { + /* Out of memory: this arrival is simply not reported (as in + hid_internal_hotplug_process_arrival) */ + continue; + } + copy->next = NULL; + hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); + hid_free_enumeration(copy); + } - while (removed != NULL) { - struct hid_device_info *info = removed; - removed = info->next; + /* LEFT for every cached usage entry whose device is gone from `fresh`, then + free the old cache. Membership is tested against `fresh`. */ + while (old != NULL) { + struct hid_device_info *info = old; + old = info->next; /* One usage entry per invocation: the callback always sees next == NULL */ info->next = NULL; - hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); + if (hid_internal_find_device_in_list(fresh, info->path) == NULL) { + hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); + } hid_free_enumeration(info); } } +/* Attempt to rebuild the udev monitor after its socket failed, then reconcile + the device cache against the current reality. Each call is all-or-nothing: on + success the context's udev monitor/fd are replaced with a fresh, receiving + monitor, *new_fd holds the new descriptor, the cache is reconciled (only real + deltas dispatched) and 0 is returned; on any failure nothing observable + changes - the old (dead) monitor and the cache are left untouched for the + caller to retry or release - and -1 is returned. The re-enumeration is quiet: + this runs on the monitor thread, which never writes the global error string + (see hidapi.h). Only ever runs on the monitor thread, with the mutex held. */ +static int hid_internal_hotplug_recover_monitor(int *new_fd) +{ + struct udev *udev_ctx; + struct udev_monitor *mon; + int fd; + int failure = 0; + struct hid_device_info *fresh; + + udev_ctx = udev_new(); + if (!udev_ctx) { + return -1; + } + mon = udev_monitor_new_from_netlink(udev_ctx, "udev"); + if (!mon + || udev_monitor_filter_add_match_subsystem_devtype(mon, "hidraw", NULL) < 0 + || udev_monitor_enable_receiving(mon) < 0 + || (fd = udev_monitor_get_fd(mon)) < 0) { + if (mon) { + udev_monitor_unref(mon); + } + udev_unref(udev_ctx); + return -1; + } + + /* Enumerate BEFORE committing anything: a failure here (a broken udev, not + an empty system) leaves the dead monitor and the cache untouched so the + caller can retry. An empty result is not a failure - it means every cached + device genuinely left, which the reconcile below reports as real LEFTs. */ + fresh = hid_internal_enumerate(0, 0, 1 /* quiet */, &failure); + if (failure) { + hid_free_enumeration(fresh); + udev_monitor_unref(mon); + udev_unref(udev_ctx); + return -1; + } + + /* Commit: swap in the fresh monitor, releasing the dead one. */ + if (hid_hotplug_context.mon) { + udev_monitor_unref(hid_hotplug_context.mon); + } + if (hid_hotplug_context.udev_ctx) { + udev_unref(hid_hotplug_context.udev_ctx); + } + hid_hotplug_context.mon = mon; + hid_hotplug_context.udev_ctx = udev_ctx; + hid_hotplug_context.monitor_fd = fd; + *new_fd = fd; + + /* Reconcile the cache with reality and resume monitoring on the new fd. */ + hid_internal_hotplug_reconcile(fresh); + return 0; +} + /* Dispatch one udev monitor event. Only ever runs on the monitor thread, with the mutex held. */ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) @@ -1680,11 +1839,18 @@ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) attempt, so a bounded number of retries filters those out. */ #define HID_HOTPLUG_SOCKET_ERROR_POLL_LIMIT 100 +/* Consecutive attempts to rebuild the udev monitor after its socket died before + the machinery gives up. Each attempt is a full udev context + monitor + recreation and a re-enumeration, paced one per loop iteration; a small bound + rides out a transient failure without spinning on a genuinely broken udev. */ +#define HID_HOTPLUG_MONITOR_RECOVER_LIMIT 5 + static void* hotplug_thread(void* user_data) { int monitor_fd; int socket_error_polls = 0; int socket_dead = 0; + int recover_attempts = 0; (void) user_data; @@ -1721,31 +1887,60 @@ static void* hotplug_thread(void* user_data) device cache, the udev monitor and its context) right away - when the last callback removed itself from within a callback, no further hotplug call is guaranteed to come and reap it - - then announce the exit under the mutex and stop touching any + then publish the exit under the mutex and stop touching any shared state. A monitoring context created after this point belongs to a new thread; the release is idempotent, so a claimant repeating it is a no-op. */ hid_internal_hotplug_release_monitor(); hid_hotplug_context.monitor_dead = 0; - /* This thread is detached: nothing is left to reap once it - announces its exit (it touches no shared state below this - point, and everything it owned has just been released) */ - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + /* Publish FINISHED as the last write under the mutex (the thread + touches no shared state below this point, and everything it owned + has just been released), then unlock and return. The pthread_t + stays JOINABLE: hid_exit() - and the next register/deregister - + join it (see hid_internal_hotplug_join_thread), so no + monitor-thread instruction is still running inside the library + once hid_exit() returns. When the last callback deregistered + itself from within a callback and the application then never calls + HIDAPI again, this one pthread_t lingers unjoined until hid_exit() + reaps it - a bounded, single-thread leak that is strictly better + than a detached thread resuming in unmapped code after dlclose(). */ + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_FINISHED; stop = 1; } else { if (socket_dead && !hid_hotplug_context.monitor_dead) { - /* The udev monitor socket is dead: no hotplug event can ever - be observed again. Fail into a well-defined state instead - of silently dropping events from now on: report every - cached device as removed (keeping the arrived/left events - paired for every callback), release the monitor, and mark - the machinery dead - new registrations are refused until - it winds down completely. The global error string is not - touched: HIDAPI's internal threads never write it (see - hidapi.h). */ - hid_hotplug_context.monitor_dead = 1; - hid_internal_hotplug_drop_all_devices(); - hid_internal_hotplug_release_monitor(); + /* The udev monitor socket died. Losing the event transport is + NOT evidence that the devices disappeared, so DO NOT fabricate + removals for the cached (still-connected) devices. Instead try + to rebuild the transport a bounded number of times; a + successful rebuild re-enumerates and dispatches only the real + deltas that occurred while the socket was down. */ + if (recover_attempts < HID_HOTPLUG_MONITOR_RECOVER_LIMIT) { + int new_fd = -1; + ++recover_attempts; + if (hid_internal_hotplug_recover_monitor(&new_fd) == 0) { + /* Transport rebuilt and cache reconciled: resume + monitoring on the fresh descriptor. */ + monitor_fd = new_fd; + socket_dead = 0; + socket_error_polls = 0; + recover_attempts = 0; + } + /* else: the dead monitor and the cache are untouched; retry + on a later iteration until the bound is reached */ + } else { + /* Unrecoverable: stop monitoring WITHOUT synthesizing any + removal - the cached devices really are still connected, + so the cache is left exactly as it is. Mark the machinery + dead so new registrations are refused + (hid_hotplug_register_callback) - that refusal is the + observable failure; the global error string is NOT written + from this thread (see hidapi.h). After this, no hotplug + event is ever delivered again for this machinery + generation: the thread idles until its callbacks are + deregistered, then winds down and releases everything + (the dead monitor included). */ + hid_hotplug_context.monitor_dead = 1; + } } /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE @@ -2002,7 +2197,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven here would leave pre-connected devices without their "left" events later, so it fails the registration (unlike an empty system, which is not an error) */ - hid_hotplug_context.devs = hid_internal_enumerate(0, 0, &enumerate_failure); + hid_hotplug_context.devs = hid_internal_enumerate(0, 0, 0, &enumerate_failure); if (!enumerate_failure) { register_global_error(NULL); } @@ -2025,22 +2220,16 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hid_hotplug_context.hotplug_cbs = hotplug_cb; /* Start the thread that will be doing the event scanning. - The thread is detached: when the last callback deregisters itself - from within a callback, the thread winds down on its own and no - further hotplug call - which is what would join it - is guaranteed - to ever come. It releases the monitoring context and announces - HID_HOTPLUG_THREAD_NONE under the mutex before it returns, so - nothing it owns or touches can outlive hid_exit(). */ - pthread_attr_t attr; - int thread_error = 1; - - if (pthread_attr_init(&attr) == 0) { - if (pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED) == 0 - && pthread_create(&hid_hotplug_context.thread, &attr, &hotplug_thread, NULL) == 0) { - thread_error = 0; - } - pthread_attr_destroy(&attr); - } + The thread is JOINABLE. When it winds down (the last callback is + gone) it releases the monitoring context and publishes + HID_HOTPLUG_THREAD_FINISHED under the mutex before it returns; the + pthread_t is then reaped by hid_exit() or the next + register/deregister (see hid_internal_hotplug_join_thread), so no + monitor-thread code is still executing inside the library once + hid_exit() returns - which a detached thread could not guarantee. + Joining is done by ANOTHER thread, never the monitor thread itself, + so it is ThreadSanitizer-clean (unlike pthread_detach(self)). */ + int thread_error = pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL); if (thread_error) { hid_hotplug_context.hotplug_cbs = NULL; From c8edb1a088521881744781cb0150412d32256d79 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 02:51:50 +0300 Subject: [PATCH 06/10] linux: harden the hotplug monitor join and recovery reconcile Follow-up review fixes, four of them in the recover/reconcile path: - Join the finished monitor thread with the mutex released and never from the thread itself, so a callback-armed TSD destructor can re-enter HIDAPI at join time without self-deadlock or self-join. - Treat a partial recovery re-enumeration as a failed attempt (a per-entry udev failure now flags it) instead of fabricating removals for present devices. - Reconcile on a stable per-connection identity, not the devnode path alone, so a device replaced on the same /dev/hidrawN yields LEFT(old) + ARRIVED(new). - Drop a reconcile arrival whose callback copy fails to allocate, so it is neither silently cached nor later reported as left. - Re-enumerate before the replacement monitor starts receiving, so a failed recovery does not discard already-queued events. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 224 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 185 insertions(+), 39 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index fe6817233..8f9f229db 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -661,7 +661,15 @@ static int parse_uevent_info(const char *uevent, unsigned *bus_type, /* quiet: don't touch the global error string - for the callers on HIDAPI's internal monitor thread, which never writes it (see hidapi.h) */ -static struct hid_device_info * create_device_info_for_device(struct udev_device *raw_dev, int quiet) +/* Build the hid_device_info chain (one entry per usage) for a single udev device + node. Returns NULL both for benign exclusions (no HID parent, an unparseable + uevent, an unhandled bus type - exactly what hid_enumerate() would also skip) + and for genuine resource failures. When `failure` is non-NULL it is set to 1 + ONLY in the latter case, so a caller that needs an authoritative, complete + enumeration (the monitor-thread reconcile) can tell a device that is + legitimately absent from one that merely failed to materialize. It is never + cleared here, so callers accumulate it across a whole enumeration. */ +static struct hid_device_info * create_device_info_for_device(struct udev_device *raw_dev, int quiet, int *failure) { struct hid_device_info *root = NULL; struct hid_device_info *cur_dev = NULL; @@ -721,8 +729,14 @@ static struct hid_device_info * create_device_info_for_device(struct udev_device /* Create the record. */ root = (struct hid_device_info*) calloc(1, sizeof(struct hid_device_info)); - if (!root) + if (!root) { + /* A genuine resource failure, unlike the benign NULL returns above: + flag it for callers that require an all-or-nothing enumeration. */ + if (failure) { + *failure = 1; + } goto end; + } cur_dev = root; @@ -855,8 +869,14 @@ static struct hid_device_info * create_device_info_for_device(struct udev_device struct hid_device_info *tmp = (struct hid_device_info*) calloc(1, sizeof(struct hid_device_info)); struct hid_device_info *prev_dev = cur_dev; - if (!tmp) + if (!tmp) { + /* Out of memory mid-device: the returned chain would be + incomplete, so flag it for all-or-nothing callers. */ + if (failure) { + *failure = 1; + } break; + } cur_dev->next = tmp; cur_dev = tmp; @@ -909,7 +929,7 @@ static struct hid_device_info * create_device_info_for_hid_device(hid_device *de /* Open a udev device from the dev_t. 'c' means character device. */ udev_dev = udev_device_new_from_devnum(udev, 'c', s.st_rdev); if (udev_dev) { - root = create_device_info_for_device(udev_dev, 0); + root = create_device_info_for_device(udev_dev, 0, NULL); } if (!root) { @@ -950,7 +970,17 @@ enum hid_hotplug_thread_state { published its exit and hid_exit() returned, it could still be running its own epilogue (unlock/return) in the library's text when the application calls dlclose(), i.e. resume in unmapped code. */ - HID_HOTPLUG_THREAD_FINISHED + HID_HOTPLUG_THREAD_FINISHED, + /* A reaper has claimed the finished thread and is joining its pthread_t + with the hotplug mutex RELEASED (see hid_internal_hotplug_join_thread). + The join must not run under the mutex: the exiting monitor thread may + still run thread-specific-data destructors that a user callback armed + while running on it, and such a destructor re-entering HIDAPI (taking the + hotplug mutex) would deadlock a joiner that held it. Publishing this + transient state under the mutex makes the claim exactly-once - a + concurrent reaper observes it and backs off rather than joining the same + pthread_t twice (which is undefined behavior). */ + HID_HOTPLUG_THREAD_JOINING }; static struct hid_hotplug_context { @@ -1059,12 +1089,20 @@ static void hid_internal_hotplug_release_monitor(void) as its last write under the mutex) but has not been joined yet, turning its state to HID_HOTPLUG_THREAD_NONE. Called with the mutex held. - The join is performed WITH the mutex held: a finished thread needs nothing - further from the mutex (it released the monitoring context and published - FINISHED before unlocking, then only runs unlock/return), so joining under - the mutex cannot deadlock and naturally serializes concurrent reapers - - whoever loses the race for the mutex observes HID_HOTPLUG_THREAD_NONE and - returns, so the pthread_t is joined exactly once. + The claim on the finished thread is made under the mutex (FINISHED -> JOINING, + an exactly-once flip), but the pthread_join() itself runs with the mutex + RELEASED, then the mutex is re-acquired and the state advanced to NONE. The + join must not hold the mutex: the finished monitor thread has already released + everything it owned and published FINISHED, so all that can still execute on + it is its epilogue and any thread-specific-data destructors a user callback + armed while running on it - and such a destructor re-entering HIDAPI would + block on the hotplug mutex forever if the joiner held it, deadlocking the + join. With the mutex released the destructor takes it, runs, and the thread + terminates so the join can complete. A concurrent reaper observes JOINING (not + FINISHED) and backs off, so the pthread_t is joined exactly once; a caller in + hid_internal_hotplug_cleanup waits out the JOINING window before it can reach + NONE. Because the mutex is dropped, callers must re-validate cached state after + this returns (hid_internal_hotplug_cleanup already does). Never runs on the monitor thread itself: the thread becomes FINISHED only after it has stopped dispatching, and the callers below are gated by @@ -1072,11 +1110,29 @@ static void hid_internal_hotplug_release_monitor(void) joiner. */ static void hid_internal_hotplug_join_thread(void) { + pthread_t thread; + if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { return; } - pthread_join(hid_hotplug_context.thread, NULL); + /* Never join from the monitor thread itself. A thread-specific-data + destructor armed by a user callback runs on the monitor thread AFTER it + published FINISHED - so mutex_in_use (a dispatch in flight) no longer + guards this path - and if that destructor re-enters HIDAPI and reaches + here, joining would be a self-join (undefined behavior / EDEADLK). Leave + the thread FINISHED for an application-thread reaper - hid_exit() or the + next register/deregister - to join. */ + if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { + return; + } + + /* Claim the join exactly-once under the mutex, then join WITHOUT it. */ + thread = hid_hotplug_context.thread; + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_JOINING; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + pthread_join(thread, NULL); + pthread_mutex_lock(&hid_hotplug_context.mutex); hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; } @@ -1114,9 +1170,12 @@ static void hid_internal_hotplug_cleanup(void) return; } - /* HID_HOTPLUG_THREAD_RUNNING: drop the mutex so the thread can acquire - it, observe the empty callback list, release the monitoring context - and publish FINISHED; then re-evaluate and join from scratch */ + /* HID_HOTPLUG_THREAD_RUNNING (the thread has not yet noticed the empty + callback list) or HID_HOTPLUG_THREAD_JOINING (another reaper is joining + the finished thread with the mutex released): drop the mutex so that + thread / that reaper can make progress, then re-evaluate. The loser of + a join race waits here until the winner publishes + HID_HOTPLUG_THREAD_NONE, so the pthread_t is joined exactly once. */ pthread_mutex_unlock(&hid_hotplug_context.mutex); poll(NULL, 0, 1); pthread_mutex_lock(&hid_hotplug_context.mutex); @@ -1386,10 +1445,21 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, } raw_dev = udev_device_new_from_syspath(udev, sysfs_path); - if (!raw_dev) + if (!raw_dev) { + /* A path the scan just listed no longer resolves to a udev device: + the enumeration is no longer a complete, authoritative snapshot. + Flag it so an all-or-nothing caller (the monitor-thread reconcile + in hid_internal_hotplug_recover_monitor) discards the partial + result instead of mistaking the missing entries for departures. + hid_enumerate() passes failure == NULL and keeps its long-standing + best-effort behavior. */ + if (failure) { + *failure = 1; + } continue; + } - tmp = create_device_info_for_device(raw_dev, quiet); + tmp = create_device_info_for_device(raw_dev, quiet, failure); if (tmp) { if (cur_dev) { cur_dev->next = tmp; @@ -1570,6 +1640,56 @@ static struct hid_device_info *hid_internal_find_device_by_path(const char *path return hid_internal_find_device_in_list(hid_hotplug_context.devs, path); } +/* NULL-safe wide-string equality (two NULL pointers compare equal). */ +static int hid_internal_wcs_equal(const wchar_t *a, const wchar_t *b) +{ + if (a == NULL || b == NULL) { + return a == b; + } + return wcscmp(a, b) == 0; +} + +/* Whether two device-info entries describe the SAME physical connection, not + merely the same devnode path. While the udev event transport is down, the + device on a given /dev/hidrawN may be unplugged and a different one plugged + into the same node: the path is then identical but the connection is not. + Reconcile must tell these apart - matching on the path alone would keep the + stale entry and miss BOTH the old device's LEFT and the new device's ARRIVED - + so it also compares the stable per-connection identity udev reports: + vid/pid/serial/release/interface/bus. Usage page and usage are deliberately + excluded, so that the several usage entries of one physical device all share a + single connection identity (they arrive and leave together). */ +static int hid_internal_same_connection(const struct hid_device_info *a, const struct hid_device_info *b) +{ + if (a->path == NULL || b->path == NULL) { + if (a->path != b->path) { + return 0; + } + } else if (strcmp(a->path, b->path) != 0) { + return 0; + } + return a->vendor_id == b->vendor_id + && a->product_id == b->product_id + && a->release_number == b->release_number + && a->interface_number == b->interface_number + && a->bus_type == b->bus_type + && hid_internal_wcs_equal(a->serial_number, b->serial_number); +} + +/* First entry in `list` describing the same physical connection as `entry`, or + NULL. Used only by reconcile (see hid_internal_same_connection); the live + add/remove paths match on the devnode path alone, since a udev "remove" event + carries only that. */ +static struct hid_device_info *hid_internal_find_same_connection(struct hid_device_info *list, const struct hid_device_info *entry) +{ + for (struct hid_device_info *device = list; device; device = device->next) { + if (hid_internal_same_connection(device, entry)) { + return device; + } + } + return NULL; +} + /* Handle a device arrival: update the connected-device cache and dispatch the callbacks. Takes ownership of the whole device chain (one entry per usage). Only ever runs on the monitor thread, with the mutex held. */ @@ -1690,6 +1810,7 @@ static void hid_internal_hotplug_reconcile(struct hid_device_info *fresh) (see hid_internal_hotplug_process_arrival). */ hid_hotplug_callback_handle dispatch_bound = hid_hotplug_context.next_handle - 1; struct hid_device_info *old = hid_hotplug_context.devs; + struct hid_device_info **link; /* Adopt the fresh enumeration as the cache up front: a callback registered from within one of the dispatches below (handle beyond dispatch_bound, so @@ -1697,33 +1818,50 @@ static void hid_internal_hotplug_reconcile(struct hid_device_info *fresh) snapshot instead - keeping every arrival/departure exactly-once. */ hid_hotplug_context.devs = fresh; - /* ARRIVED for every fresh usage entry whose device was not cached. Membership - is tested against the still-intact `old` cache. A COPY is dispatched (one - entry, next == NULL) so the adopted cache chain stays walkable. */ - for (struct hid_device_info *device = fresh; device != NULL; device = device->next) { + /* ARRIVED for every fresh usage entry whose connection was not cached. + Membership is tested against the still-intact `old` cache on the stable + connection identity, NOT the devnode path alone: a device swapped for a + different one on the same /dev/hidrawN during the outage is thus reported + as ARRIVED(new) here and LEFT(old) below, instead of being mistaken for the + unchanged presence of the old device. A COPY (one entry, next == NULL) is + dispatched so the adopted cache chain stays walkable. If the copy cannot be + allocated the arrival cannot be announced, so the entry is DROPPED from the + adopted cache as well: leaving it in would later fabricate a LEFT for a + device the callbacks never saw arrive. */ + link = &hid_hotplug_context.devs; + while (*link != NULL) { + struct hid_device_info *device = *link; struct hid_device_info *copy; - if (hid_internal_find_device_in_list(old, device->path) != NULL) { + if (hid_internal_find_same_connection(old, device) != NULL) { + link = &device->next; continue; } copy = hid_internal_copy_device_info(device); if (copy == NULL) { - /* Out of memory: this arrival is simply not reported (as in - hid_internal_hotplug_process_arrival) */ + /* Out of memory: neither announce nor silently keep this arrival. */ + *link = device->next; + device->next = NULL; + hid_free_enumeration(device); continue; } copy->next = NULL; hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); hid_free_enumeration(copy); + link = &device->next; } - /* LEFT for every cached usage entry whose device is gone from `fresh`, then - free the old cache. Membership is tested against `fresh`. */ + /* LEFT for every cached usage entry whose connection is gone from the + reconciled cache, then free the old cache. Membership is tested against the + adopted cache (hid_hotplug_context.devs, not the possibly-dangling `fresh` + head after a drop above) on the same connection identity. A dropped-on-OOM + arrival was never in `old`, so it can never match one of these old entries - + testing against the reduced cache cannot miss a real departure. */ while (old != NULL) { struct hid_device_info *info = old; old = info->next; /* One usage entry per invocation: the callback always sees next == NULL */ info->next = NULL; - if (hid_internal_find_device_in_list(fresh, info->path) == NULL) { + if (hid_internal_find_same_connection(hid_hotplug_context.devs, info) == NULL) { hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); } hid_free_enumeration(info); @@ -1747,8 +1885,27 @@ static int hid_internal_hotplug_recover_monitor(int *new_fd) int failure = 0; struct hid_device_info *fresh; + /* Re-enumerate FIRST, off an independent udev context, BEFORE any + replacement monitor starts receiving. Doing it in this order means a + monitor is enabled for receiving only on a path that goes on to commit, so + a monitor that has begun queueing real events is never built and then + thrown away (which would discard those events). A failure here - a broken + udev, or a partial/non-authoritative enumeration (see + hid_internal_enumerate): reconciling against an incomplete set would + fabricate departures for still-present devices - aborts the attempt with + the dead monitor and the cache left untouched for the caller to retry. An + empty result is NOT a failure: it means every cached device genuinely left, + which the reconcile below reports as real LEFTs. Quiet: this runs on the + monitor thread, which never writes the global error string (see hidapi.h). */ + fresh = hid_internal_enumerate(0, 0, 1 /* quiet */, &failure); + if (failure) { + hid_free_enumeration(fresh); + return -1; + } + udev_ctx = udev_new(); if (!udev_ctx) { + hid_free_enumeration(fresh); return -1; } mon = udev_monitor_new_from_netlink(udev_ctx, "udev"); @@ -1760,18 +1917,7 @@ static int hid_internal_hotplug_recover_monitor(int *new_fd) udev_monitor_unref(mon); } udev_unref(udev_ctx); - return -1; - } - - /* Enumerate BEFORE committing anything: a failure here (a broken udev, not - an empty system) leaves the dead monitor and the cache untouched so the - caller can retry. An empty result is not a failure - it means every cached - device genuinely left, which the reconcile below reports as real LEFTs. */ - fresh = hid_internal_enumerate(0, 0, 1 /* quiet */, &failure); - if (failure) { hid_free_enumeration(fresh); - udev_monitor_unref(mon); - udev_unref(udev_ctx); return -1; } @@ -1808,7 +1954,7 @@ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) invisible to hid_enumerate(); there is nothing meaningful to deliver instead. quiet: this runs on the monitor thread, which never writes the global error string (see hidapi.h). */ - hid_internal_hotplug_process_arrival(create_device_info_for_device(raw_dev, 1)); + hid_internal_hotplug_process_arrival(create_device_info_for_device(raw_dev, 1, NULL)); } else if (!strcmp(action, "remove")) { const char *devnode = udev_device_get_devnode(raw_dev); char devnode_buf[32]; From 77d6cae4adad775c9c49e4c5e8c313418786c1f8 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 03:25:42 +0300 Subject: [PATCH 07/10] linux/hidraw: drop hotplug monitor recovery; fix the thread-join lifecycle Remove the recover-and-reconcile machinery for a dead udev monitor socket (hid_internal_hotplug_recover_monitor / _reconcile / _same_connection / _find_same_connection and the wide-string helper). No other backend attempts recovery, and the approach carried inherent flaws (all-or-nothing re-enumeration; connection identity cannot distinguish two serial-less devices reusing the same /dev/hidrawN). On an unrecoverable POLLERR/POLLHUP/POLLNVAL the monitor thread now stops delivering events cleanly: it fabricates no DEVICE_LEFT, leaves the device cache and the application's open handles untouched, and sets monitor_dead so further hid_hotplug_register_callback() calls are refused. The bounded ENOBUFS poll retry is kept. Rework the monitor-thread reap to the join_in_progress-flag + condition-variable pattern. The thread runs joinable and publishes FINISHED as its last write; a reaper on an app thread sets join_in_progress, joins with the mutex released, then re-acquires, publishes NONE and broadcasts. A second reaper waits on the condvar instead of double-joining, and a register waiting to start a new thread waits out any in-flight join before reusing the pthread_t. A re-entrant reap from the monitor thread itself (a callback-armed TSD destructor) defers via pthread_equal; hid_exit() performs any outstanding join so the thread is provably gone once it returns. hid_internal_enumerate loses its now-unused quiet parameter; the failure out-parameter is retained for the initial-registration snapshot. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 497 ++++++++++++++++------------------------------------ 1 file changed, 152 insertions(+), 345 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 8f9f229db..ded799e05 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -665,8 +665,9 @@ static int parse_uevent_info(const char *uevent, unsigned *bus_type, node. Returns NULL both for benign exclusions (no HID parent, an unparseable uevent, an unhandled bus type - exactly what hid_enumerate() would also skip) and for genuine resource failures. When `failure` is non-NULL it is set to 1 - ONLY in the latter case, so a caller that needs an authoritative, complete - enumeration (the monitor-thread reconcile) can tell a device that is + ONLY in the latter case, so a caller that needs an all-or-nothing enumeration + (the initial hotplug-registration snapshot, which must fail rather than arm the + callbacks against an incomplete device set) can tell a device that is legitimately absent from one that merely failed to materialize. It is never cleared here, so callers accumulate it across a whole enumeration. */ static struct hid_device_info * create_device_info_for_device(struct udev_device *raw_dev, int quiet, int *failure) @@ -961,26 +962,20 @@ enum hid_hotplug_thread_state { HID_HOTPLUG_THREAD_NONE, /* The monitor thread is running */ HID_HOTPLUG_THREAD_RUNNING, - /* The monitor thread has returned - it published this state as its last - write under the mutex, immediately before unlocking and returning - but - its pthread_t has not been joined yet. The thread is JOINABLE, not - detached: hid_exit() (and the next register/deregister) join it, so once - hid_exit() returns no monitor-thread instruction can still be executing - inside the library. A detached thread could not guarantee that - after it - published its exit and hid_exit() returned, it could still be running its - own epilogue (unlock/return) in the library's text when the application - calls dlclose(), i.e. resume in unmapped code. */ - HID_HOTPLUG_THREAD_FINISHED, - /* A reaper has claimed the finished thread and is joining its pthread_t - with the hotplug mutex RELEASED (see hid_internal_hotplug_join_thread). - The join must not run under the mutex: the exiting monitor thread may - still run thread-specific-data destructors that a user callback armed - while running on it, and such a destructor re-entering HIDAPI (taking the - hotplug mutex) would deadlock a joiner that held it. Publishing this - transient state under the mutex makes the claim exactly-once - a - concurrent reaper observes it and backs off rather than joining the same - pthread_t twice (which is undefined behavior). */ - HID_HOTPLUG_THREAD_JOINING + /* The monitor thread has returned - it published this state as its LAST + write to shared state under the mutex, immediately before unlocking and + returning - but its pthread_t has not been joined yet. The thread is + JOINABLE, not detached: hid_exit() (and the next register/deregister) + reap it (see hid_internal_hotplug_reap_thread), so once hid_exit() returns + no monitor-thread instruction can still be executing inside the library. A + detached thread could not guarantee that - after it published its exit and + hid_exit() returned, it could still be running its own epilogue + (unlock/return) in the library's text when the application calls + dlclose(), i.e. resume in unmapped code. The reap is coordinated by the + join_in_progress flag and the thread_cond condition variable rather than a + distinct state, so a concurrent reaper waits for the join instead of + joining the same pthread_t twice. */ + HID_HOTPLUG_THREAD_FINISHED }; static struct hid_hotplug_context { @@ -1000,6 +995,12 @@ static struct hid_hotplug_context { pthread_mutex_t mutex; + /* Signaled when a monitor-thread reap completes (thread_state returns to + NONE and join_in_progress clears): lets a second reaper wait out an + in-flight join instead of joining the same pthread_t twice. Process- + lifetime, like the mutex: never destroyed. */ + pthread_cond_t thread_cond; + /* Boolean flags */ /* The one-time initialization of the mutex succeeded (written once, under pthread_once); the mutex lives for the rest of the process */ @@ -1008,6 +1009,10 @@ static struct hid_hotplug_context { unsigned char cb_list_dirty; /* hid_exit() is tearing the hotplug machinery down */ unsigned char exiting; + /* A reaper has claimed the FINISHED monitor thread and is joining its + pthread_t with the mutex released; set/cleared under the mutex, cleared + with a thread_cond broadcast (see hid_internal_hotplug_reap_thread) */ + unsigned char join_in_progress; /* The udev monitor socket died; no more events (see hotplug_thread) */ unsigned char monitor_dead; @@ -1086,63 +1091,70 @@ static void hid_internal_hotplug_release_monitor(void) } /* Reaps a monitor thread that has finished (published HID_HOTPLUG_THREAD_FINISHED - as its last write under the mutex) but has not been joined yet, turning its - state to HID_HOTPLUG_THREAD_NONE. Called with the mutex held. - - The claim on the finished thread is made under the mutex (FINISHED -> JOINING, - an exactly-once flip), but the pthread_join() itself runs with the mutex - RELEASED, then the mutex is re-acquired and the state advanced to NONE. The - join must not hold the mutex: the finished monitor thread has already released - everything it owned and published FINISHED, so all that can still execute on - it is its epilogue and any thread-specific-data destructors a user callback - armed while running on it - and such a destructor re-entering HIDAPI would - block on the hotplug mutex forever if the joiner held it, deadlocking the - join. With the mutex released the destructor takes it, runs, and the thread - terminates so the join can complete. A concurrent reaper observes JOINING (not - FINISHED) and backs off, so the pthread_t is joined exactly once; a caller in - hid_internal_hotplug_cleanup waits out the JOINING window before it can reach - NONE. Because the mutex is dropped, callers must re-validate cached state after - this returns (hid_internal_hotplug_cleanup already does). - - Never runs on the monitor thread itself: the thread becomes FINISHED only - after it has stopped dispatching, and the callers below are gated by - mutex_in_use (a dispatch in flight), so a thread can never be its own - joiner. */ -static void hid_internal_hotplug_join_thread(void) + as its last write to shared state under the mutex) but has not been joined yet, + turning its state to HID_HOTPLUG_THREAD_NONE. Called with the mutex held + exactly once. + + pthread_join() must NOT run under the mutex: the finished monitor thread has + already released everything it owned and published FINISHED, but its epilogue + and any thread-specific-data destructors a user callback armed while running on + it may still execute - and such a destructor re-entering HIDAPI would block on + the hotplug mutex forever if the joiner held it, deadlocking the join. So the + reaper sets join_in_progress, RELEASES the mutex for the join, then RE-ACQUIRES + it, advances the state to NONE, clears join_in_progress and broadcasts + thread_cond. A second reaper that finds join_in_progress set waits on + thread_cond instead of joining the same pthread_t twice (undefined behavior), + and wakes to observe NONE. Because the mutex is dropped, callers must + re-validate cached state after this returns (hid_internal_hotplug_cleanup + does). + + The monitor thread never joins itself: a thread-specific-data destructor armed + by a user callback runs on the monitor thread AFTER it published FINISHED (so + mutex_in_use no longer guards this path), and if that destructor re-enters + HIDAPI and reaches here a self-join would be undefined behavior / EDEADLK. The + pthread_equal guard makes such a re-entrant attempt defer, leaving the thread + FINISHED for an application-thread reaper - hid_exit() or the next + register/deregister. */ +static void hid_internal_hotplug_reap_thread(void) { pthread_t thread; - if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { + /* A re-entrant reap from the monitor thread itself cannot join itself: + defer to the next application-thread reaper / hid_exit(). */ + if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { return; } - /* Never join from the monitor thread itself. A thread-specific-data - destructor armed by a user callback runs on the monitor thread AFTER it - published FINISHED - so mutex_in_use (a dispatch in flight) no longer - guards this path - and if that destructor re-enters HIDAPI and reaches - here, joining would be a self-join (undefined behavior / EDEADLK). Leave - the thread FINISHED for an application-thread reaper - hid_exit() or the - next register/deregister - to join. */ - if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { + /* Another reaper already claimed the join: wait it out (thread_cond is + broadcast when the join completes) instead of joining twice. */ + while (hid_hotplug_context.join_in_progress) { + pthread_cond_wait(&hid_hotplug_context.thread_cond, &hid_hotplug_context.mutex); + } + + if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { + /* Nothing to reap: never finished, or the reaper we waited on already + advanced it to NONE. */ return; } - /* Claim the join exactly-once under the mutex, then join WITHOUT it. */ + /* Claim the join, drop the mutex for it, then re-acquire and publish NONE. */ thread = hid_hotplug_context.thread; - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_JOINING; + hid_hotplug_context.join_in_progress = 1; pthread_mutex_unlock(&hid_hotplug_context.mutex); pthread_join(thread, NULL); pthread_mutex_lock(&hid_hotplug_context.mutex); hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + hid_hotplug_context.join_in_progress = 0; + pthread_cond_broadcast(&hid_hotplug_context.thread_cond); } -/* Winds the monitor thread down once the last callback is gone and joins it, so +/* Winds the monitor thread down once the last callback is gone and reaps it, so that no monitor-thread code is still executing once this returns. The thread - releases the monitoring context itself, then publishes FINISHED; this joins - the FINISHED thread. Called with the mutex held exactly once (and not in - use); the mutex is temporarily dropped while waiting for a still-RUNNING - thread to notice the empty list, so the caller must re-validate any cached - state after this returns. */ + releases the monitoring context itself, then publishes FINISHED; this reaps + the FINISHED thread (see hid_internal_hotplug_reap_thread). Called with the + mutex held exactly once (and not in use); the mutex is temporarily dropped + while waiting for a still-RUNNING thread to notice the empty list and while + joining, so the caller must re-validate any cached state after this returns. */ static void hid_internal_hotplug_cleanup(void) { if (hid_hotplug_context.mutex_in_use) { @@ -1165,17 +1177,24 @@ static void hid_internal_hotplug_cleanup(void) } if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { - /* The thread released the monitoring context and returned: join it */ - hid_internal_hotplug_join_thread(); - return; + /* On the monitor thread itself (a thread-specific-data destructor + re-entering HIDAPI after FINISHED was published) the reap defers + without joining - leave the pthread_t for an application-thread + reaper / hid_exit() and return, rather than spin here. */ + if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { + return; + } + /* The thread released the monitoring context and returned: reap it + (joining, or waiting out a concurrent reaper on thread_cond, both + with the mutex released), then re-evaluate. With hotplug_cbs still + NULL no new thread can have appeared, so the loop settles on NONE. */ + hid_internal_hotplug_reap_thread(); + continue; } - /* HID_HOTPLUG_THREAD_RUNNING (the thread has not yet noticed the empty - callback list) or HID_HOTPLUG_THREAD_JOINING (another reaper is joining - the finished thread with the mutex released): drop the mutex so that - thread / that reaper can make progress, then re-evaluate. The loser of - a join race waits here until the winner publishes - HID_HOTPLUG_THREAD_NONE, so the pthread_t is joined exactly once. */ + /* HID_HOTPLUG_THREAD_RUNNING: the thread has not yet noticed the empty + callback list. Drop the mutex so it can make progress towards + publishing FINISHED, then re-evaluate. */ pthread_mutex_unlock(&hid_hotplug_context.mutex); poll(NULL, 0, 1); pthread_mutex_lock(&hid_hotplug_context.mutex); @@ -1206,6 +1225,13 @@ static void hid_internal_hotplug_init_once(void) } pthread_mutexattr_destroy(&attr); + /* Process-lifetime, like the mutex (never destroyed): coordinates the + monitor-thread reap so concurrent reapers never double-join. */ + if (pthread_cond_init(&hid_hotplug_context.thread_cond, NULL) != 0) { + pthread_mutex_destroy(&hid_hotplug_context.mutex); + return; + } + hid_hotplug_context.monitor_fd = -1; /* The handles are monotonic and never reused (see hidapi.h); the counter survives hid_exit */ @@ -1346,14 +1372,11 @@ static int hid_internal_match_device_id(unsigned short vendor_id, unsigned short } /* Same as hid_enumerate, but distinguishes a genuine failure from an empty - system: *failure (when non-NULL) is set to 1 only when the enumeration - itself failed (with the global error set accordingly). */ -/* quiet: don't touch the global error string - for the caller on HIDAPI's - internal monitor thread (the hotplug re-enumeration after a monitor rebuild), - which never writes it (see hidapi.h). A quiet caller must already have - initialized the library, so hid_init() is skipped too (it resets the global - error). An empty result is not a failure in either mode. */ -static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, unsigned short product_id, int quiet, int *failure) + system: *failure (when non-NULL) is set to 1 only when the enumeration itself + failed (with the global error set accordingly). An empty result is not a + failure. Used by the initial hotplug-registration snapshot, which must fail + rather than arm the callbacks against an incomplete device set. */ +static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, unsigned short product_id, int *failure) { struct udev *udev; struct udev_enumerate *enumerate; @@ -1366,17 +1389,13 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, *failure = 0; } - if (!quiet) { - hid_init(); - /* register_global_error: global error is reset by hid_init */ - } + hid_init(); + /* register_global_error: global error is reset by hid_init */ /* Create the udev object */ udev = udev_new(); if (!udev) { - if (!quiet) { - register_global_error("Couldn't create udev context"); - } + register_global_error("Couldn't create udev context"); if (failure) { *failure = 1; } @@ -1387,9 +1406,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, enumerate = udev_enumerate_new(udev); if (!enumerate) { udev_unref(udev); - if (!quiet) { - register_global_error("Couldn't create udev enumeration"); - } + register_global_error("Couldn't create udev enumeration"); if (failure) { *failure = 1; } @@ -1398,9 +1415,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) { udev_enumerate_unref(enumerate); udev_unref(udev); - if (!quiet) { - register_global_error("Couldn't add the hidraw subsystem match to the udev enumeration"); - } + register_global_error("Couldn't add the hidraw subsystem match to the udev enumeration"); if (failure) { *failure = 1; } @@ -1409,9 +1424,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (udev_enumerate_scan_devices(enumerate) < 0) { udev_enumerate_unref(enumerate); udev_unref(udev); - if (!quiet) { - register_global_error("Couldn't scan the udev devices"); - } + register_global_error("Couldn't scan the udev devices"); if (failure) { *failure = 1; } @@ -1448,18 +1461,18 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, if (!raw_dev) { /* A path the scan just listed no longer resolves to a udev device: the enumeration is no longer a complete, authoritative snapshot. - Flag it so an all-or-nothing caller (the monitor-thread reconcile - in hid_internal_hotplug_recover_monitor) discards the partial - result instead of mistaking the missing entries for departures. - hid_enumerate() passes failure == NULL and keeps its long-standing - best-effort behavior. */ + Flag it so an all-or-nothing caller (the initial hotplug- + registration snapshot) discards the partial result instead of + mistaking the missing entries for departures. hid_enumerate() + passes failure == NULL and keeps its long-standing best-effort + behavior. */ if (failure) { *failure = 1; } continue; } - tmp = create_device_info_for_device(raw_dev, quiet, failure); + tmp = create_device_info_for_device(raw_dev, 0, failure); if (tmp) { if (cur_dev) { cur_dev->next = tmp; @@ -1481,7 +1494,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, udev_enumerate_unref(enumerate); udev_unref(udev); - if (root == NULL && !quiet) { + if (root == NULL) { if (vendor_id == 0 && product_id == 0) { register_global_error("No HID devices found in the system."); } else { @@ -1494,7 +1507,7 @@ static struct hid_device_info *hid_internal_enumerate(unsigned short vendor_id, struct hid_device_info HID_API_EXPORT *hid_enumerate(unsigned short vendor_id, unsigned short product_id) { - return hid_internal_enumerate(vendor_id, product_id, 0, NULL); + return hid_internal_enumerate(vendor_id, product_id, NULL); } void HID_API_EXPORT hid_free_enumeration(struct hid_device_info *devs) @@ -1640,56 +1653,6 @@ static struct hid_device_info *hid_internal_find_device_by_path(const char *path return hid_internal_find_device_in_list(hid_hotplug_context.devs, path); } -/* NULL-safe wide-string equality (two NULL pointers compare equal). */ -static int hid_internal_wcs_equal(const wchar_t *a, const wchar_t *b) -{ - if (a == NULL || b == NULL) { - return a == b; - } - return wcscmp(a, b) == 0; -} - -/* Whether two device-info entries describe the SAME physical connection, not - merely the same devnode path. While the udev event transport is down, the - device on a given /dev/hidrawN may be unplugged and a different one plugged - into the same node: the path is then identical but the connection is not. - Reconcile must tell these apart - matching on the path alone would keep the - stale entry and miss BOTH the old device's LEFT and the new device's ARRIVED - - so it also compares the stable per-connection identity udev reports: - vid/pid/serial/release/interface/bus. Usage page and usage are deliberately - excluded, so that the several usage entries of one physical device all share a - single connection identity (they arrive and leave together). */ -static int hid_internal_same_connection(const struct hid_device_info *a, const struct hid_device_info *b) -{ - if (a->path == NULL || b->path == NULL) { - if (a->path != b->path) { - return 0; - } - } else if (strcmp(a->path, b->path) != 0) { - return 0; - } - return a->vendor_id == b->vendor_id - && a->product_id == b->product_id - && a->release_number == b->release_number - && a->interface_number == b->interface_number - && a->bus_type == b->bus_type - && hid_internal_wcs_equal(a->serial_number, b->serial_number); -} - -/* First entry in `list` describing the same physical connection as `entry`, or - NULL. Used only by reconcile (see hid_internal_same_connection); the live - add/remove paths match on the devnode path alone, since a udev "remove" event - carries only that. */ -static struct hid_device_info *hid_internal_find_same_connection(struct hid_device_info *list, const struct hid_device_info *entry) -{ - for (struct hid_device_info *device = list; device; device = device->next) { - if (hid_internal_same_connection(device, entry)) { - return device; - } - } - return NULL; -} - /* Handle a device arrival: update the connected-device cache and dispatch the callbacks. Takes ownership of the whole device chain (one entry per usage). Only ever runs on the monitor thread, with the mutex held. */ @@ -1796,148 +1759,6 @@ static void hid_internal_hotplug_process_removal(const char *devnode) } } -/* Reconcile the device cache with a fresh enumeration after the udev monitor - was rebuilt, delivering ONLY the real deltas that happened while the transport - was down: DEVICE_ARRIVED for a device present now but not cached, DEVICE_LEFT - for a cached device genuinely absent now. A device present in both is left - untouched - no event is fabricated for a device that never went anywhere. - The fresh enumeration is adopted as the new cache. Takes ownership of `fresh` - and of the previous cache. Only ever runs on the monitor thread, with the - mutex held. */ -static void hid_internal_hotplug_reconcile(struct hid_device_info *fresh) -{ - /* Freeze the dispatch to the callbacks registered up to this point - (see hid_internal_hotplug_process_arrival). */ - hid_hotplug_callback_handle dispatch_bound = hid_hotplug_context.next_handle - 1; - struct hid_device_info *old = hid_hotplug_context.devs; - struct hid_device_info **link; - - /* Adopt the fresh enumeration as the cache up front: a callback registered - from within one of the dispatches below (handle beyond dispatch_bound, so - excluded from them) then captures the reconciled set in its ENUMERATE - snapshot instead - keeping every arrival/departure exactly-once. */ - hid_hotplug_context.devs = fresh; - - /* ARRIVED for every fresh usage entry whose connection was not cached. - Membership is tested against the still-intact `old` cache on the stable - connection identity, NOT the devnode path alone: a device swapped for a - different one on the same /dev/hidrawN during the outage is thus reported - as ARRIVED(new) here and LEFT(old) below, instead of being mistaken for the - unchanged presence of the old device. A COPY (one entry, next == NULL) is - dispatched so the adopted cache chain stays walkable. If the copy cannot be - allocated the arrival cannot be announced, so the entry is DROPPED from the - adopted cache as well: leaving it in would later fabricate a LEFT for a - device the callbacks never saw arrive. */ - link = &hid_hotplug_context.devs; - while (*link != NULL) { - struct hid_device_info *device = *link; - struct hid_device_info *copy; - if (hid_internal_find_same_connection(old, device) != NULL) { - link = &device->next; - continue; - } - copy = hid_internal_copy_device_info(device); - if (copy == NULL) { - /* Out of memory: neither announce nor silently keep this arrival. */ - *link = device->next; - device->next = NULL; - hid_free_enumeration(device); - continue; - } - copy->next = NULL; - hid_internal_invoke_callbacks(copy, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED, dispatch_bound); - hid_free_enumeration(copy); - link = &device->next; - } - - /* LEFT for every cached usage entry whose connection is gone from the - reconciled cache, then free the old cache. Membership is tested against the - adopted cache (hid_hotplug_context.devs, not the possibly-dangling `fresh` - head after a drop above) on the same connection identity. A dropped-on-OOM - arrival was never in `old`, so it can never match one of these old entries - - testing against the reduced cache cannot miss a real departure. */ - while (old != NULL) { - struct hid_device_info *info = old; - old = info->next; - /* One usage entry per invocation: the callback always sees next == NULL */ - info->next = NULL; - if (hid_internal_find_same_connection(hid_hotplug_context.devs, info) == NULL) { - hid_internal_invoke_callbacks(info, HID_API_HOTPLUG_EVENT_DEVICE_LEFT, dispatch_bound); - } - hid_free_enumeration(info); - } -} - -/* Attempt to rebuild the udev monitor after its socket failed, then reconcile - the device cache against the current reality. Each call is all-or-nothing: on - success the context's udev monitor/fd are replaced with a fresh, receiving - monitor, *new_fd holds the new descriptor, the cache is reconciled (only real - deltas dispatched) and 0 is returned; on any failure nothing observable - changes - the old (dead) monitor and the cache are left untouched for the - caller to retry or release - and -1 is returned. The re-enumeration is quiet: - this runs on the monitor thread, which never writes the global error string - (see hidapi.h). Only ever runs on the monitor thread, with the mutex held. */ -static int hid_internal_hotplug_recover_monitor(int *new_fd) -{ - struct udev *udev_ctx; - struct udev_monitor *mon; - int fd; - int failure = 0; - struct hid_device_info *fresh; - - /* Re-enumerate FIRST, off an independent udev context, BEFORE any - replacement monitor starts receiving. Doing it in this order means a - monitor is enabled for receiving only on a path that goes on to commit, so - a monitor that has begun queueing real events is never built and then - thrown away (which would discard those events). A failure here - a broken - udev, or a partial/non-authoritative enumeration (see - hid_internal_enumerate): reconciling against an incomplete set would - fabricate departures for still-present devices - aborts the attempt with - the dead monitor and the cache left untouched for the caller to retry. An - empty result is NOT a failure: it means every cached device genuinely left, - which the reconcile below reports as real LEFTs. Quiet: this runs on the - monitor thread, which never writes the global error string (see hidapi.h). */ - fresh = hid_internal_enumerate(0, 0, 1 /* quiet */, &failure); - if (failure) { - hid_free_enumeration(fresh); - return -1; - } - - udev_ctx = udev_new(); - if (!udev_ctx) { - hid_free_enumeration(fresh); - return -1; - } - mon = udev_monitor_new_from_netlink(udev_ctx, "udev"); - if (!mon - || udev_monitor_filter_add_match_subsystem_devtype(mon, "hidraw", NULL) < 0 - || udev_monitor_enable_receiving(mon) < 0 - || (fd = udev_monitor_get_fd(mon)) < 0) { - if (mon) { - udev_monitor_unref(mon); - } - udev_unref(udev_ctx); - hid_free_enumeration(fresh); - return -1; - } - - /* Commit: swap in the fresh monitor, releasing the dead one. */ - if (hid_hotplug_context.mon) { - udev_monitor_unref(hid_hotplug_context.mon); - } - if (hid_hotplug_context.udev_ctx) { - udev_unref(hid_hotplug_context.udev_ctx); - } - hid_hotplug_context.mon = mon; - hid_hotplug_context.udev_ctx = udev_ctx; - hid_hotplug_context.monitor_fd = fd; - *new_fd = fd; - - /* Reconcile the cache with reality and resume monitoring on the new fd. */ - hid_internal_hotplug_reconcile(fresh); - return 0; -} - /* Dispatch one udev monitor event. Only ever runs on the monitor thread, with the mutex held. */ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) @@ -1985,18 +1806,11 @@ static void hid_internal_hotplug_process_event(struct udev_device *raw_dev) attempt, so a bounded number of retries filters those out. */ #define HID_HOTPLUG_SOCKET_ERROR_POLL_LIMIT 100 -/* Consecutive attempts to rebuild the udev monitor after its socket died before - the machinery gives up. Each attempt is a full udev context + monitor - recreation and a re-enumeration, paced one per loop iteration; a small bound - rides out a transient failure without spinning on a genuinely broken udev. */ -#define HID_HOTPLUG_MONITOR_RECOVER_LIMIT 5 - static void* hotplug_thread(void* user_data) { int monitor_fd; int socket_error_polls = 0; int socket_dead = 0; - int recover_attempts = 0; (void) user_data; @@ -2043,7 +1857,7 @@ static void* hotplug_thread(void* user_data) touches no shared state below this point, and everything it owned has just been released), then unlock and return. The pthread_t stays JOINABLE: hid_exit() - and the next register/deregister - - join it (see hid_internal_hotplug_join_thread), so no + reap it (see hid_internal_hotplug_reap_thread), so no monitor-thread instruction is still running inside the library once hid_exit() returns. When the last callback deregistered itself from within a callback and the application then never calls @@ -2054,39 +1868,25 @@ static void* hotplug_thread(void* user_data) stop = 1; } else { if (socket_dead && !hid_hotplug_context.monitor_dead) { - /* The udev monitor socket died. Losing the event transport is - NOT evidence that the devices disappeared, so DO NOT fabricate - removals for the cached (still-connected) devices. Instead try - to rebuild the transport a bounded number of times; a - successful rebuild re-enumerates and dispatches only the real - deltas that occurred while the socket was down. */ - if (recover_attempts < HID_HOTPLUG_MONITOR_RECOVER_LIMIT) { - int new_fd = -1; - ++recover_attempts; - if (hid_internal_hotplug_recover_monitor(&new_fd) == 0) { - /* Transport rebuilt and cache reconciled: resume - monitoring on the fresh descriptor. */ - monitor_fd = new_fd; - socket_dead = 0; - socket_error_polls = 0; - recover_attempts = 0; - } - /* else: the dead monitor and the cache are untouched; retry - on a later iteration until the bound is reached */ - } else { - /* Unrecoverable: stop monitoring WITHOUT synthesizing any - removal - the cached devices really are still connected, - so the cache is left exactly as it is. Mark the machinery - dead so new registrations are refused - (hid_hotplug_register_callback) - that refusal is the - observable failure; the global error string is NOT written - from this thread (see hidapi.h). After this, no hotplug - event is ever delivered again for this machinery - generation: the thread idles until its callbacks are - deregistered, then winds down and releases everything - (the dead monitor included). */ - hid_hotplug_context.monitor_dead = 1; - } + /* The udev monitor socket died unrecoverably (see the poll() + handling below). Stop delivering events cleanly: losing the + event transport is NOT evidence that the devices left, so do + NOT fabricate removals and do NOT re-enumerate or diff. The + device cache is left exactly as it is and the application's + open handles are untouched (those devices are still physically + connected). Mark the machinery dead so new registrations are + refused (hid_hotplug_register_callback) - that refusal is the + observable failure; the global error string is NOT written + from this thread (see hidapi.h). + + After an unrecoverable monitor failure, no further hotplug + event is delivered for this machinery generation; devices + already reported remain in the cache and open handles are + unaffected. The thread keeps idling (still flushing any pending + initial ENUMERATE passes) until its callbacks are deregistered, + then winds down and releases everything (the dead monitor + included). */ + hid_hotplug_context.monitor_dead = 1; } /* Deliver the pending initial passes of HID_API_HOTPLUG_ENUMERATE @@ -2343,7 +2143,7 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven here would leave pre-connected devices without their "left" events later, so it fails the registration (unlike an empty system, which is not an error) */ - hid_hotplug_context.devs = hid_internal_enumerate(0, 0, 0, &enumerate_failure); + hid_hotplug_context.devs = hid_internal_enumerate(0, 0, &enumerate_failure); if (!enumerate_failure) { register_global_error(NULL); } @@ -2366,15 +2166,22 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hid_hotplug_context.hotplug_cbs = hotplug_cb; /* Start the thread that will be doing the event scanning. - The thread is JOINABLE. When it winds down (the last callback is - gone) it releases the monitoring context and publishes + hid_internal_hotplug_cleanup() above ran with the callback list empty, + so it wound any previous monitor thread all the way down to + HID_HOTPLUG_THREAD_NONE - reaping a FINISHED one and waiting out any + in-flight join on thread_cond - before we reach here. The state is + therefore NONE and the mutex has been held continuously since, so this + pthread_create never overwrites a context.thread that still labels a + live or unreaped thread. + The thread is JOINABLE. When it winds down (the last callback is gone) + it releases the monitoring context and publishes HID_HOTPLUG_THREAD_FINISHED under the mutex before it returns; the - pthread_t is then reaped by hid_exit() or the next - register/deregister (see hid_internal_hotplug_join_thread), so no - monitor-thread code is still executing inside the library once - hid_exit() returns - which a detached thread could not guarantee. - Joining is done by ANOTHER thread, never the monitor thread itself, - so it is ThreadSanitizer-clean (unlike pthread_detach(self)). */ + pthread_t is then reaped by hid_exit() or the next register/deregister + (see hid_internal_hotplug_reap_thread), so no monitor-thread code is + still executing inside the library once hid_exit() returns - which a + detached thread could not guarantee. Joining is done by ANOTHER thread, + never the monitor thread itself, so it is ThreadSanitizer-clean (unlike + pthread_detach(self)). */ int thread_error = pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL); if (thread_error) { From e202abfc49760b3ea8b0397210ea926787754266 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 03:57:21 +0300 Subject: [PATCH 08/10] linux: don't reap a new monitor generation started during the join window The hotplug reaper releases the mutex to pthread_join() a FINISHED monitor thread. A thread-specific-data destructor running on that very thread can re-enter hid_hotplug_register_callback() during the join (the pthread_equal self-guard lets it proceed instead of cond-waiting), which finds the callback list empty and starts a NEW monitor generation, overwriting context.thread and setting thread_state = RUNNING. On re-acquiring the mutex the reaper then unconditionally stamped thread_state = NONE, clobbering that live new generation: hid_exit() would see NONE and return without joining it, leaving a monitor thread running inside the library after it is unloaded (the dlclose use-after-free). Advance to NONE only when context.thread still names the thread this reaper joined (by identity, pthread_equal) and it is still FINISHED; otherwise a newer generation now owns the lifecycle state and is left untouched (reaped by its own future reaper / hid_exit). join_in_progress is still cleared and thread_cond broadcast unconditionally, so any reaper or registration waiting out the join wakes and re-evaluates. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index ded799e05..2a236f2b8 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -1101,8 +1101,10 @@ static void hid_internal_hotplug_release_monitor(void) it may still execute - and such a destructor re-entering HIDAPI would block on the hotplug mutex forever if the joiner held it, deadlocking the join. So the reaper sets join_in_progress, RELEASES the mutex for the join, then RE-ACQUIRES - it, advances the state to NONE, clears join_in_progress and broadcasts - thread_cond. A second reaper that finds join_in_progress set waits on + it, advances the state to NONE - but ONLY IF the generation it joined is still + the current one (such a destructor may have started a new generation while the + mutex was dropped; see the join site below) - then clears join_in_progress and + broadcasts thread_cond. A second reaper that finds join_in_progress set waits on thread_cond instead of joining the same pthread_t twice (undefined behavior), and wakes to observe NONE. Because the mutex is dropped, callers must re-validate cached state after this returns (hid_internal_hotplug_cleanup @@ -1137,13 +1139,36 @@ static void hid_internal_hotplug_reap_thread(void) return; } - /* Claim the join, drop the mutex for it, then re-acquire and publish NONE. */ + /* Claim the join, drop the mutex for it, then re-acquire. */ thread = hid_hotplug_context.thread; hid_hotplug_context.join_in_progress = 1; pthread_mutex_unlock(&hid_hotplug_context.mutex); pthread_join(thread, NULL); pthread_mutex_lock(&hid_hotplug_context.mutex); - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + + /* Publish NONE only if the generation just joined is still the current one. + While the mutex was dropped for the join, a thread-specific-data + destructor running on the very thread being joined can - legally, from a + user-callback context - re-enter hid_hotplug_register_callback(): its + hid_internal_hotplug_cleanup() takes the pthread_equal() self-guard and + DEFERS instead of joining, so the registration proceeds, finds the + callback list empty and starts a NEW monitor generation, overwriting + context.thread with a live thread and setting thread_state = RUNNING. + Stamping NONE over that would strand the new generation: hid_exit() would + see NONE and return WITHOUT joining it, leaving a monitor thread running + inside the library after it is unloaded (the dlclose use-after-free). So + advance to NONE only when context.thread still names the thread this + reaper joined (by identity - pthread_equal, not ==) and it is still + FINISHED; otherwise a newer generation now owns the lifecycle state and + must be left completely untouched (it is reaped by its own future reaper / + hid_exit). The completed pthread_join() already reaped the old thread, so + nothing leaks either way. join_in_progress is cleared and thread_cond + broadcast unconditionally, so any reaper or registration waiting out this + join wakes and re-evaluates whichever branch was taken. */ + if (pthread_equal(hid_hotplug_context.thread, thread) + && hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + } hid_hotplug_context.join_in_progress = 0; pthread_cond_broadcast(&hid_hotplug_context.thread_cond); } From bfe82aa24d985b639aa69e2349a14024fe8d8bc1 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 13:57:08 +0300 Subject: [PATCH 09/10] linux: track monitor threads on a retired list so none is dropped Give the hotplug machinery a retired-thread list so a monitor generation that is superseded by a re-entrant registration - a callback's TSD destructor running on the just-finished monitor thread starts a new generation - is never dropped and is always joined before hid_exit() returns. The reaper now moves a FINISHED generation onto the list (keyed by a stable monotonic id token) and then joins it with the mutex released, so a pthread_t is never inspected after pthread_join() invalidated it. hid_exit() joins every retired generation and the current one. A per-entry being_joined claim replaces join_in_progress; the exiting guard, clean stop on monitor death, quiet error suppression on the monitor thread and the single lock order are unchanged. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 371 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 231 insertions(+), 140 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 2a236f2b8..922e6895d 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -971,13 +971,38 @@ enum hid_hotplug_thread_state { detached thread could not guarantee that - after it published its exit and hid_exit() returned, it could still be running its own epilogue (unlock/return) in the library's text when the application calls - dlclose(), i.e. resume in unmapped code. The reap is coordinated by the - join_in_progress flag and the thread_cond condition variable rather than a - distinct state, so a concurrent reaper waits for the join instead of - joining the same pthread_t twice. */ + dlclose(), i.e. resume in unmapped code. Reaping first RETIRES the finished + generation (moves its pthread_t onto hid_hotplug_context.retired, keyed by a + stable id token) and then joins it with the mutex released, so a pthread_t + is never inspected after pthread_join() invalidated it, and a generation + superseded by a re-entrant registration is joined rather than dropped. */ HID_HOTPLUG_THREAD_FINISHED }; +/* A monitor thread that has been CREATED (pthread_create succeeded) and not yet + JOINED. The current generation lives in hid_hotplug_context.thread / + .thread_id / .thread_state; once it publishes FINISHED it is moved onto + hid_hotplug_context.retired (see hid_internal_hotplug_reap_thread) and joined + from there. A generation that is superseded while still FINISHED-but-unjoined + - a thread-specific-data destructor running on it re-enters HIDAPI and starts a + new generation - is therefore never dropped: its pthread_t stays on this list + until joined. Threads are identified by the monotonic `id` token and by list + membership, NEVER by comparing a pthread_t after it was joined (that handle is + invalid). hid_exit() joins EVERY entry on this list (and the current thread) + before it returns. */ +struct hid_hotplug_monitor_thread { + pthread_t thread; + /* Stable identity token, assigned at creation (hid_hotplug_context.thread_id + at the time). Survives the join; used for identity/diagnostics without ever + touching the joined pthread_t. */ + unsigned long id; + /* A reaper has set this and dropped the mutex to pthread_join() this entry: + any other reaper skips it (a second join of the same pthread_t is undefined + behavior), and hid_exit() waits on thread_cond for the join to finish. */ + unsigned char being_joined; + struct hid_hotplug_monitor_thread *next; +}; + static struct hid_hotplug_context { /* UDEV context that handles the monitor */ struct udev* udev_ctx; @@ -988,17 +1013,31 @@ static struct hid_hotplug_context { /* File descriptor for the UDEV monitor that allows to check for new events with poll() */ int monitor_fd; - /* Thread for the UDEV monitor */ + /* Thread for the UDEV monitor (the current generation) */ pthread_t thread; + /* Stable identity token of the current generation's thread (0 = none). + Assigned from next_thread_id at each pthread_create so a generation can be + identified without ever comparing a joined pthread_t. */ + unsigned long thread_id; + + /* Monotonic source of thread_id tokens; never reused, survives hid_exit */ + unsigned long next_thread_id; + enum hid_hotplug_thread_state thread_state; + /* Monitor threads created but not yet joined that are no longer the current + generation (superseded/orphaned). Their pthread_t is moved here instead of + being dropped; every entry is joined before hid_exit() returns. */ + struct hid_hotplug_monitor_thread *retired; + pthread_mutex_t mutex; - /* Signaled when a monitor-thread reap completes (thread_state returns to - NONE and join_in_progress clears): lets a second reaper wait out an - in-flight join instead of joining the same pthread_t twice. Process- - lifetime, like the mutex: never destroyed. */ + /* Broadcast whenever a retired monitor thread is joined and removed from the + list. hid_exit() must join every generation before returning; when the only + entries left are being joined by another (pre-exit) reaper, it waits here + for that join to complete instead of double-joining. Process-lifetime, like + the mutex: never destroyed. */ pthread_cond_t thread_cond; /* Boolean flags */ @@ -1009,10 +1048,6 @@ static struct hid_hotplug_context { unsigned char cb_list_dirty; /* hid_exit() is tearing the hotplug machinery down */ unsigned char exiting; - /* A reaper has claimed the FINISHED monitor thread and is joining its - pthread_t with the mutex released; set/cleared under the mutex, cleared - with a thread_cond broadcast (see hid_internal_hotplug_reap_thread) */ - unsigned char join_in_progress; /* The udev monitor socket died; no more events (see hotplug_thread) */ unsigned char monitor_dead; @@ -1090,96 +1125,115 @@ static void hid_internal_hotplug_release_monitor(void) hid_hotplug_context.monitor_fd = -1; } -/* Reaps a monitor thread that has finished (published HID_HOTPLUG_THREAD_FINISHED - as its last write to shared state under the mutex) but has not been joined yet, - turning its state to HID_HOTPLUG_THREAD_NONE. Called with the mutex held - exactly once. - - pthread_join() must NOT run under the mutex: the finished monitor thread has - already released everything it owned and published FINISHED, but its epilogue - and any thread-specific-data destructors a user callback armed while running on - it may still execute - and such a destructor re-entering HIDAPI would block on - the hotplug mutex forever if the joiner held it, deadlocking the join. So the - reaper sets join_in_progress, RELEASES the mutex for the join, then RE-ACQUIRES - it, advances the state to NONE - but ONLY IF the generation it joined is still - the current one (such a destructor may have started a new generation while the - mutex was dropped; see the join site below) - then clears join_in_progress and - broadcasts thread_cond. A second reaper that finds join_in_progress set waits on - thread_cond instead of joining the same pthread_t twice (undefined behavior), - and wakes to observe NONE. Because the mutex is dropped, callers must - re-validate cached state after this returns (hid_internal_hotplug_cleanup - does). - - The monitor thread never joins itself: a thread-specific-data destructor armed - by a user callback runs on the monitor thread AFTER it published FINISHED (so - mutex_in_use no longer guards this path), and if that destructor re-enters - HIDAPI and reaches here a self-join would be undefined behavior / EDEADLK. The - pthread_equal guard makes such a re-entrant attempt defer, leaving the thread - FINISHED for an application-thread reaper - hid_exit() or the next - register/deregister. */ -static void hid_internal_hotplug_reap_thread(void) +/* Moves the current generation onto the retired list once it has published + FINISHED, so its pthread_t is tracked (never dropped) and the current slot is + free for a new generation. The generation is identified by its stable id token, + not by its pthread_t. Called with the mutex held. Returns -1 only if the + (small) retired-list node cannot be allocated - leaving the thread FINISHED for + a later retry; it never drops or overwrites a live pthread_t. */ +static int hid_internal_hotplug_retire_current(void) { - pthread_t thread; + struct hid_hotplug_monitor_thread *node; - /* A re-entrant reap from the monitor thread itself cannot join itself: - defer to the next application-thread reaper / hid_exit(). */ - if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { - return; + if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { + return 0; } - /* Another reaper already claimed the join: wait it out (thread_cond is - broadcast when the join completes) instead of joining twice. */ - while (hid_hotplug_context.join_in_progress) { - pthread_cond_wait(&hid_hotplug_context.thread_cond, &hid_hotplug_context.mutex); + node = (struct hid_hotplug_monitor_thread *)calloc(1, sizeof(*node)); + if (node == NULL) { + return -1; } + node->thread = hid_hotplug_context.thread; + node->id = hid_hotplug_context.thread_id; + node->being_joined = 0; + node->next = hid_hotplug_context.retired; + hid_hotplug_context.retired = node; - if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { - /* Nothing to reap: never finished, or the reaper we waited on already - advanced it to NONE. */ - return; + /* The current slot no longer names a live-or-finished thread. */ + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + hid_hotplug_context.thread_id = 0; + return 0; +} + +/* Reaps monitor threads: first RETIRES the current generation if it has finished + (hid_internal_hotplug_retire_current), then joins every retired thread it is + allowed to join, each with the mutex RELEASED. Called with the mutex held + exactly once; because the mutex is dropped for the joins, callers must + re-validate any cached state after this returns. + + pthread_join() must NOT run under the mutex: a finished monitor thread has + published FINISHED and released everything it owned, but a thread-specific-data + destructor armed by a user callback still runs on it afterwards - and such a + destructor re-entering HIDAPI would block on the hotplug mutex forever if the + joiner held it, deadlocking the join. So each entry is claimed (being_joined), + the mutex is dropped, the thread is joined, the mutex is re-acquired, and the + entry is unlinked and freed with a thread_cond broadcast. + + Both hazards are handled WITHOUT ever inspecting a joined pthread_t: + - Self: a destructor re-entering HIDAPI on the monitor thread reaches here; a + thread cannot join itself. Its entry is skipped (pthread_self() compared + against the still-valid, unjoined handle) and left on the list for an + application-thread reaper / hid_exit(). + - Concurrent reapers: an entry already being joined (being_joined) is skipped, + so the same pthread_t is never joined twice; its claimant unlinks it when its + join completes. Identity is by list membership and the id token - a pthread_t + is only ever passed to pthread_equal()/pthread_join() while still unjoined. */ +static void hid_internal_hotplug_reap_thread(void) +{ + /* Retire the finished current generation so it is tracked on the list and + joined below (or by a later reaper / hid_exit). On allocation failure it + stays FINISHED: no pthread_t is dropped, and the pthread_create() site + refuses to overwrite a non-NONE slot, so no generation is orphaned. */ + (void)hid_internal_hotplug_retire_current(); + + for (;;) { + struct hid_hotplug_monitor_thread **link; + struct hid_hotplug_monitor_thread *node = NULL; + + for (link = &hid_hotplug_context.retired; *link != NULL; link = &(*link)->next) { + if (!(*link)->being_joined + && !pthread_equal(pthread_self(), (*link)->thread)) { + node = *link; + break; + } + } + if (node == NULL) { + /* Nothing left that we may join: the list is empty, or the only + entries are ourselves or already being joined by another reaper. */ + return; + } + + /* Claim the join, drop the mutex for it, then re-acquire and unlink. */ + node->being_joined = 1; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + pthread_join(node->thread, NULL); + pthread_mutex_lock(&hid_hotplug_context.mutex); + + for (link = &hid_hotplug_context.retired; *link != NULL; link = &(*link)->next) { + if (*link == node) { + *link = node->next; + break; + } + } + free(node); + /* Wake hid_exit() (or any reaper) waiting for this join to complete. */ + pthread_cond_broadcast(&hid_hotplug_context.thread_cond); } +} - /* Claim the join, drop the mutex for it, then re-acquire. */ - thread = hid_hotplug_context.thread; - hid_hotplug_context.join_in_progress = 1; - pthread_mutex_unlock(&hid_hotplug_context.mutex); - pthread_join(thread, NULL); - pthread_mutex_lock(&hid_hotplug_context.mutex); +/* Winds the monitor thread down once the last callback is gone and reaps it. The + thread releases the monitoring context itself, then publishes FINISHED; this + retires it and joins the retired threads it can (see + hid_internal_hotplug_reap_thread). Called with the mutex held exactly once (and + not in use); the mutex is temporarily dropped while waiting for a still-RUNNING + thread to notice the empty list and while joining, so the caller must + re-validate any cached state after this returns. - /* Publish NONE only if the generation just joined is still the current one. - While the mutex was dropped for the join, a thread-specific-data - destructor running on the very thread being joined can - legally, from a - user-callback context - re-enter hid_hotplug_register_callback(): its - hid_internal_hotplug_cleanup() takes the pthread_equal() self-guard and - DEFERS instead of joining, so the registration proceeds, finds the - callback list empty and starts a NEW monitor generation, overwriting - context.thread with a live thread and setting thread_state = RUNNING. - Stamping NONE over that would strand the new generation: hid_exit() would - see NONE and return WITHOUT joining it, leaving a monitor thread running - inside the library after it is unloaded (the dlclose use-after-free). So - advance to NONE only when context.thread still names the thread this - reaper joined (by identity - pthread_equal, not ==) and it is still - FINISHED; otherwise a newer generation now owns the lifecycle state and - must be left completely untouched (it is reaped by its own future reaper / - hid_exit). The completed pthread_join() already reaped the old thread, so - nothing leaks either way. join_in_progress is cleared and thread_cond - broadcast unconditionally, so any reaper or registration waiting out this - join wakes and re-evaluates whichever branch was taken. */ - if (pthread_equal(hid_hotplug_context.thread, thread) - && hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; - } - hid_hotplug_context.join_in_progress = 0; - pthread_cond_broadcast(&hid_hotplug_context.thread_cond); -} - -/* Winds the monitor thread down once the last callback is gone and reaps it, so - that no monitor-thread code is still executing once this returns. The thread - releases the monitoring context itself, then publishes FINISHED; this reaps - the FINISHED thread (see hid_internal_hotplug_reap_thread). Called with the - mutex held exactly once (and not in use); the mutex is temporarily dropped - while waiting for a still-RUNNING thread to notice the empty list and while - joining, so the caller must re-validate any cached state after this returns. */ + This does NOT guarantee every retired generation is joined before it returns: + an entry that is the calling (monitor) thread, or one another reaper is + joining, is left on the list. A plain register/deregister must not block on + another generation's teardown; hid_exit() is the backstop that joins EVERY + generation before it returns. */ static void hid_internal_hotplug_cleanup(void) { if (hid_hotplug_context.mutex_in_use) { @@ -1195,34 +1249,24 @@ static void hid_internal_hotplug_cleanup(void) return; } - if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { - /* No thread to join: it never started, or a concurrent caller - already reaped it (and may have started a new one) */ - return; - } - - if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { - /* On the monitor thread itself (a thread-specific-data destructor - re-entering HIDAPI after FINISHED was published) the reap defers - without joining - leave the pthread_t for an application-thread - reaper / hid_exit() and return, rather than spin here. */ - if (pthread_equal(pthread_self(), hid_hotplug_context.thread)) { - return; - } - /* The thread released the monitoring context and returned: reap it - (joining, or waiting out a concurrent reaper on thread_cond, both - with the mutex released), then re-evaluate. With hotplug_cbs still - NULL no new thread can have appeared, so the loop settles on NONE. */ - hid_internal_hotplug_reap_thread(); + if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_RUNNING) { + /* The thread has not yet noticed the empty callback list. Drop the + mutex so it can make progress towards publishing FINISHED, then + re-evaluate. */ + pthread_mutex_unlock(&hid_hotplug_context.mutex); + poll(NULL, 0, 1); + pthread_mutex_lock(&hid_hotplug_context.mutex); continue; } - /* HID_HOTPLUG_THREAD_RUNNING: the thread has not yet noticed the empty - callback list. Drop the mutex so it can make progress towards - publishing FINISHED, then re-evaluate. */ - pthread_mutex_unlock(&hid_hotplug_context.mutex); - poll(NULL, 0, 1); - pthread_mutex_lock(&hid_hotplug_context.mutex); + /* NONE or FINISHED: retire the finished current generation and join the + retired threads we can (all with the mutex released inside the reap). + With hotplug_cbs still NULL no new generation can appear here, so this + settles - the current slot becomes NONE, or it stays FINISHED only + because it is the calling thread or a retired-node allocation failed, + and in both cases there is nothing more this caller can do. */ + hid_internal_hotplug_reap_thread(); + return; } } @@ -1250,8 +1294,8 @@ static void hid_internal_hotplug_init_once(void) } pthread_mutexattr_destroy(&attr); - /* Process-lifetime, like the mutex (never destroyed): coordinates the - monitor-thread reap so concurrent reapers never double-join. */ + /* Process-lifetime, like the mutex (never destroyed): lets hid_exit() wait for + a retired thread another reaper is joining, so no pthread_t is joined twice. */ if (pthread_cond_init(&hid_hotplug_context.thread_cond, NULL) != 0) { pthread_mutex_destroy(&hid_hotplug_context.mutex); return; @@ -1261,6 +1305,9 @@ static void hid_internal_hotplug_init_once(void) /* The handles are monotonic and never reused (see hidapi.h); the counter survives hid_exit */ hid_hotplug_context.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE; + /* Monotonic monitor-thread generation tokens; never reused, survive hid_exit. + Start at 1 so that 0 unambiguously means "no current generation". */ + hid_hotplug_context.next_thread_id = 1; /* Publish the mutex as usable, last */ hid_hotplug_context.mutex_ready = 1; @@ -1337,16 +1384,41 @@ static void hid_internal_hotplug_exit(void) } hid_hotplug_context.cb_list_dirty = 0; - /* Reap the monitor thread (temporarily dropping the mutex) and - release the monitoring context. Repeat until the teardown is - complete: `exiting` keeps anything from re-arming the machinery - while the mutex is dropped, and a concurrent deregistration may - claim the join itself - in which case the thread is observed - already reaped (HID_HOTPLUG_THREAD_NONE) here */ + /* Wind the current generation down and reap what can be reaped + (temporarily dropping the mutex). `exiting` keeps anything from + re-arming the machinery or starting a new generation while the mutex is + dropped, so from here the retired list only ever shrinks. */ hid_internal_hotplug_cleanup(); + /* Backstop for a current generation cleanup could not retire because the + (tiny) retired-list node would not allocate: join it in place. Safe + precisely because `exiting` is set - no thread-specific-data destructor + can start a new generation over the slot while the mutex is dropped, and + hid_exit() is never the monitor thread - so after the join the slot + still names the joined thread and is simply cleared. */ + if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { + pthread_t thread = hid_hotplug_context.thread; + pthread_mutex_unlock(&hid_hotplug_context.mutex); + pthread_join(thread, NULL); + pthread_mutex_lock(&hid_hotplug_context.mutex); + hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; + hid_hotplug_context.thread_id = 0; + continue; + } + + /* EVERY generation must be joined before hid_exit() returns. cleanup's + reap joined every retired thread it could; anything still on the list is + being joined by another (pre-exit) reaper - wait for that join to + broadcast thread_cond, then re-evaluate. (hid_exit() is never the + monitor thread, so no retired entry is ever "self" here.) */ + if (hid_hotplug_context.retired != NULL) { + pthread_cond_wait(&hid_hotplug_context.thread_cond, &hid_hotplug_context.mutex); + continue; + } + if (hid_hotplug_context.hotplug_cbs == NULL - && hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE) { + && hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_NONE + && hid_hotplug_context.retired == NULL) { break; } } @@ -2192,21 +2264,37 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven /* Start the thread that will be doing the event scanning. hid_internal_hotplug_cleanup() above ran with the callback list empty, - so it wound any previous monitor thread all the way down to - HID_HOTPLUG_THREAD_NONE - reaping a FINISHED one and waiting out any - in-flight join on thread_cond - before we reach here. The state is - therefore NONE and the mutex has been held continuously since, so this - pthread_create never overwrites a context.thread that still labels a - live or unreaped thread. + so it retired any finished predecessor generation onto the retired list + (joining it there, or leaving it for hid_exit()) and drove the current + slot to HID_HOTPLUG_THREAD_NONE - even in the orphan case where a + thread-specific-data destructor re-registers on the very monitor thread + that just finished: that generation is RETIRED (self-skipped for the + join), never dropped. The mutex has been held continuously since, so the + slot is NONE here; the guard below only fires if retiring the + predecessor could not allocate its (tiny) list node, in which case the + registration fails rather than overwrite - and thereby orphan - a + pthread_t that is still unjoined. The thread is JOINABLE. When it winds down (the last callback is gone) it releases the monitoring context and publishes HID_HOTPLUG_THREAD_FINISHED under the mutex before it returns; the - pthread_t is then reaped by hid_exit() or the next register/deregister - (see hid_internal_hotplug_reap_thread), so no monitor-thread code is - still executing inside the library once hid_exit() returns - which a - detached thread could not guarantee. Joining is done by ANOTHER thread, - never the monitor thread itself, so it is ThreadSanitizer-clean (unlike - pthread_detach(self)). */ + pthread_t is then retired and joined by hid_exit() or the next + register/deregister (see hid_internal_hotplug_reap_thread), so no + monitor-thread code is still executing inside the library once + hid_exit() returns - which a detached thread could not guarantee. + Joining is done by ANOTHER thread, never the monitor thread itself, so + it is ThreadSanitizer-clean (unlike pthread_detach(self)). */ + if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_NONE + && hid_internal_hotplug_retire_current() != 0) { + hid_hotplug_context.hotplug_cbs = NULL; + /* The handle never became visible: return it to the counter */ + hid_hotplug_context.next_handle--; + hid_internal_hotplug_release_monitor(); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + free(hotplug_cb); + register_global_error("Couldn't retire the previous hotplug monitor thread"); + return -1; + } + int thread_error = pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL); if (thread_error) { @@ -2219,6 +2307,9 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven register_global_error("Couldn't create the hotplug monitor thread"); return -1; } + /* Stamp the new generation with a fresh stable id token, then publish it + RUNNING (both under the mutex, before the thread can do anything). */ + hid_hotplug_context.thread_id = hid_hotplug_context.next_thread_id++; hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_RUNNING; } From 72b00d7e814cb04ee00637d10e7c11e7e2158797 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Wed, 15 Jul 2026 14:36:19 +0300 Subject: [PATCH 10/10] linux/hid.c: pre-allocate the retired monitor node at thread creation Reserve each monitor generation's retired-list node with a calloc at pthread_create time and let the generation own that node for its whole life, so hid_internal_hotplug_retire_current() only splices the already-owned node onto the retired list - a pointer move that can never fail. A finished generation is therefore always retired and joined. This removes the last OOM race: hid_exit()'s in-place-join backstop, which kept the current slot published while it dropped the mutex to pthread_join (racing a pre-exit reaper already inside the cleanup loop into a double-join or a post-join pthread_equal), is deleted as now unreachable. On node-allocation failure the registration fails cleanly without creating the thread, so no orphaned, unjoined generation is ever possible. Assisted-by: claude-code:claude-opus-4-8 --- linux/hid.c | 94 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/linux/hid.c b/linux/hid.c index 922e6895d..be9b5dc29 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -1026,6 +1026,15 @@ static struct hid_hotplug_context { enum hid_hotplug_thread_state thread_state; + /* The retired-list node the current generation owns, pre-allocated at + pthread_create time and non-NULL exactly while thread_state is RUNNING or + FINISHED. hid_internal_hotplug_retire_current() splices this already-owned + node onto `retired` instead of allocating, so retiring a finished + generation can never fail - which is why hid_exit() needs no in-place-join + backstop. Freed when its thread is finally joined (reaped off `retired`), + or on the register failure path if the thread is never created. */ + struct hid_hotplug_monitor_thread *thread_node; + /* Monitor threads created but not yet joined that are no longer the current generation (superseded/orphaned). Their pthread_t is moved here instead of being dropped; every entry is joined before hid_exit() returns. */ @@ -1128,21 +1137,22 @@ static void hid_internal_hotplug_release_monitor(void) /* Moves the current generation onto the retired list once it has published FINISHED, so its pthread_t is tracked (never dropped) and the current slot is free for a new generation. The generation is identified by its stable id token, - not by its pthread_t. Called with the mutex held. Returns -1 only if the - (small) retired-list node cannot be allocated - leaving the thread FINISHED for - a later retry; it never drops or overwrites a live pthread_t. */ -static int hid_internal_hotplug_retire_current(void) + not by its pthread_t. Called with the mutex held. This only splices the + generation's OWN pre-allocated node (hid_hotplug_context.thread_node, reserved + at pthread_create time) onto the list - no allocation - so it CANNOT fail and + never drops or overwrites a live pthread_t. */ +static void hid_internal_hotplug_retire_current(void) { struct hid_hotplug_monitor_thread *node; if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_FINISHED) { - return 0; + return; } - node = (struct hid_hotplug_monitor_thread *)calloc(1, sizeof(*node)); - if (node == NULL) { - return -1; - } + /* Splice the generation's own pre-allocated node (never NULL while a + generation exists) onto the retired list: a pointer move, no allocation, + so this can never fail. */ + node = hid_hotplug_context.thread_node; node->thread = hid_hotplug_context.thread; node->id = hid_hotplug_context.thread_id; node->being_joined = 0; @@ -1150,9 +1160,9 @@ static int hid_internal_hotplug_retire_current(void) hid_hotplug_context.retired = node; /* The current slot no longer names a live-or-finished thread. */ + hid_hotplug_context.thread_node = NULL; hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; hid_hotplug_context.thread_id = 0; - return 0; } /* Reaps monitor threads: first RETIRES the current generation if it has finished @@ -1181,10 +1191,10 @@ static int hid_internal_hotplug_retire_current(void) static void hid_internal_hotplug_reap_thread(void) { /* Retire the finished current generation so it is tracked on the list and - joined below (or by a later reaper / hid_exit). On allocation failure it - stays FINISHED: no pthread_t is dropped, and the pthread_create() site - refuses to overwrite a non-NONE slot, so no generation is orphaned. */ - (void)hid_internal_hotplug_retire_current(); + joined below (or by a later reaper / hid_exit). This only splices the + generation's pre-allocated node onto the list, so it cannot fail: a + finished generation is always retired and never left FINISHED here. */ + hid_internal_hotplug_retire_current(); for (;;) { struct hid_hotplug_monitor_thread **link; @@ -1390,21 +1400,11 @@ static void hid_internal_hotplug_exit(void) dropped, so from here the retired list only ever shrinks. */ hid_internal_hotplug_cleanup(); - /* Backstop for a current generation cleanup could not retire because the - (tiny) retired-list node would not allocate: join it in place. Safe - precisely because `exiting` is set - no thread-specific-data destructor - can start a new generation over the slot while the mutex is dropped, and - hid_exit() is never the monitor thread - so after the join the slot - still names the joined thread and is simply cleared. */ - if (hid_hotplug_context.thread_state == HID_HOTPLUG_THREAD_FINISHED) { - pthread_t thread = hid_hotplug_context.thread; - pthread_mutex_unlock(&hid_hotplug_context.mutex); - pthread_join(thread, NULL); - pthread_mutex_lock(&hid_hotplug_context.mutex); - hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_NONE; - hid_hotplug_context.thread_id = 0; - continue; - } + /* No in-place-join backstop is needed here: retiring a finished + generation only splices its pre-allocated node onto the retired list + and can never fail, so hid_internal_hotplug_cleanup() above always + drove the current slot to NONE (the finished generation is now on the + retired list) rather than leaving a published FINISHED slot to join. */ /* EVERY generation must be joined before hid_exit() returns. cleanup's reap joined every retired thread it could; anything still on the list is @@ -2270,10 +2270,10 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven thread-specific-data destructor re-registers on the very monitor thread that just finished: that generation is RETIRED (self-skipped for the join), never dropped. The mutex has been held continuously since, so the - slot is NONE here; the guard below only fires if retiring the - predecessor could not allocate its (tiny) list node, in which case the - registration fails rather than overwrite - and thereby orphan - a - pthread_t that is still unjoined. + slot is NONE here. The new generation's retired-list node is + pre-allocated below BEFORE the thread is created, so retiring a finished + generation only splices an already-owned node onto the list and can never + fail - which is why hid_exit() needs no in-place-join backstop. The thread is JOINABLE. When it winds down (the last callback is gone) it releases the monitoring context and publishes HID_HOTPLUG_THREAD_FINISHED under the mutex before it returns; the @@ -2283,21 +2283,37 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven hid_exit() returns - which a detached thread could not guarantee. Joining is done by ANOTHER thread, never the monitor thread itself, so it is ThreadSanitizer-clean (unlike pthread_detach(self)). */ - if (hid_hotplug_context.thread_state != HID_HOTPLUG_THREAD_NONE - && hid_internal_hotplug_retire_current() != 0) { + /* Pre-allocate the retired-list node this new generation will own for its + whole life, BEFORE creating the thread. Retiring a finished generation + (hid_internal_hotplug_retire_current) then only splices this + already-owned node onto the retired list and can never fail, so a + finished generation is ALWAYS tracked and joined and hid_exit() needs no + in-place-join backstop. An allocation failure here is handled cleanly by + failing the registration WITHOUT creating the thread: no orphan is + possible, and there is nothing to unwind beyond the normal path. */ + struct hid_hotplug_monitor_thread *thread_node = + (struct hid_hotplug_monitor_thread *)calloc(1, sizeof(*thread_node)); + if (thread_node == NULL) { hid_hotplug_context.hotplug_cbs = NULL; /* The handle never became visible: return it to the counter */ hid_hotplug_context.next_handle--; hid_internal_hotplug_release_monitor(); pthread_mutex_unlock(&hid_hotplug_context.mutex); free(hotplug_cb); - register_global_error("Couldn't retire the previous hotplug monitor thread"); + register_global_error("Couldn't allocate the hotplug monitor thread record"); return -1; } + /* Retire any finished predecessor generation onto the retired list (a + pointer splice of its own pre-allocated node - cannot fail). After the + cleanup above the slot is NONE in the common case, so this usually + no-ops; it never overwrites or drops an unjoined pthread_t. */ + hid_internal_hotplug_retire_current(); + int thread_error = pthread_create(&hid_hotplug_context.thread, NULL, &hotplug_thread, NULL); if (thread_error) { + free(thread_node); hid_hotplug_context.hotplug_cbs = NULL; /* The handle never became visible: return it to the counter */ hid_hotplug_context.next_handle--; @@ -2307,8 +2323,10 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven register_global_error("Couldn't create the hotplug monitor thread"); return -1; } - /* Stamp the new generation with a fresh stable id token, then publish it - RUNNING (both under the mutex, before the thread can do anything). */ + /* The new generation now owns its pre-allocated retired-list node. Stamp + it with a fresh stable id token, then publish it RUNNING (all under the + mutex, before the thread can do anything). */ + hid_hotplug_context.thread_node = thread_node; hid_hotplug_context.thread_id = hid_hotplug_context.next_thread_id++; hid_hotplug_context.thread_state = HID_HOTPLUG_THREAD_RUNNING; }