From a49f0e4f70fd4ae569c3a8f49718b055f06be130 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Sun, 8 Nov 2026 21:29:54 +0200 Subject: [PATCH 1/3] libusb: fail hid_open_path() if the read thread can't be started hidapi_thread_create() discarded the pthread_create() result, so hidapi_initialize_device() went on to wait on the two-party startup barrier even when no read thread existed, and hid_open_path() blocked forever (e.g. on EAGAIN under thread/resource limits). hidapi_thread_create() now returns the creation status (0 on success). On failure, hidapi_initialize_device() skips the startup barrier and the join, and goes through the existing error path that destroys the input ring, releases the interface and reattaches the kernel driver; hid_open_path() then reports "failed to initialize device". A custom thread model provided via HIDAPI_THREAD_MODEL_INCLUDE must now declare hidapi_thread_create() as returning int (0 on success, non-zero on failure). Add LibUsbReadThreadStartupTest, which compiles libusb/hid.c with pthread and device-level libusb calls interposed to cover both the injected creation failure and normal startup/close, without hardware. Fixes: #842 Related: #825, #829 Coverity CID: 1706123 Assisted-by: claude-code:claude-opus-5-5 --- libusb/hid.c | 10 +- libusb/hidapi_thread_pthread.h | 7 +- libusb/test/CMakeLists.txt | 18 + libusb/test/read_thread_startup_test.c | 443 +++++++++++++++++++++++++ 4 files changed, 475 insertions(+), 3 deletions(-) create mode 100644 libusb/test/read_thread_startup_test.c diff --git a/libusb/hid.c b/libusb/hid.c index 981d5c2ff..3ccf40cf3 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -53,6 +53,9 @@ #include "../src/core/hidapi_input_ring.h" #include "hidapi_libusb_report_descriptor.h" +/* A custom thread model (HIDAPI_THREAD_MODEL_INCLUDE) must provide the same + interface as hidapi_thread_pthread.h. In particular, hidapi_thread_create() + returns int: 0 on success, non-zero if the thread could not be started. */ #ifndef HIDAPI_THREAD_MODEL_INCLUDE #define HIDAPI_THREAD_MODEL_INCLUDE "hidapi_thread_pthread.h" #endif @@ -1465,7 +1468,12 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa goto err_release_interface; } - hidapi_thread_create(&dev->thread_state, read_thread, dev); + if (hidapi_thread_create(&dev->thread_state, read_thread, dev) != 0) { + /* Nothing would ever release the startup barrier below, and there is + no thread to join: fail the open and undo what was set up above. */ + LOG("hidapi_initialize_device: couldn't start the read thread\n"); + goto err_release_interface; + } /* Wait here for the read thread to be initialized. */ hidapi_thread_barrier_wait(&dev->thread_state); diff --git a/libusb/hidapi_thread_pthread.h b/libusb/hidapi_thread_pthread.h index 0abe733e5..52d3cc68f 100644 --- a/libusb/hidapi_thread_pthread.h +++ b/libusb/hidapi_thread_pthread.h @@ -148,9 +148,12 @@ static void hidapi_thread_barrier_wait(hidapi_thread_state *state) pthread_barrier_wait(&state->barrier); } -static void hidapi_thread_create(hidapi_thread_state *state, void *(*func)(void*), void *func_arg) +/* Starts `func` on a new thread. Returns 0 on success, or a non-zero value when + the thread could not be started (in which case `state->thread` is left unset + and must not be joined). */ +static int hidapi_thread_create(hidapi_thread_state *state, void *(*func)(void*), void *func_arg) { - pthread_create(&state->thread, NULL, func, func_arg); + return pthread_create(&state->thread, NULL, func, func_arg); } static void hidapi_thread_join(hidapi_thread_state *state) diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index ca5d4a914..f946819be 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -58,3 +58,21 @@ foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES}) WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" ) endforeach() + +# Read-thread startup: libusb/hid.c is compiled into the test, with pthread and +# device-level libusb calls interposed, so no hardware is needed. Built with the +# same definitions and libraries as the hidapi_libusb library itself. +add_executable(read_thread_startup_test read_thread_startup_test.c) +hidapi_configure_sanitizers(read_thread_startup_test) +if(HIDAPI_BUILD_AS_CXX) + set_source_files_properties(read_thread_startup_test.c PROPERTIES LANGUAGE CXX) +endif() +get_target_property(HIDAPI_LIBUSB_DEFINITIONS hidapi_libusb COMPILE_DEFINITIONS) +if(HIDAPI_LIBUSB_DEFINITIONS) + target_compile_definitions(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_DEFINITIONS}) +endif() +get_target_property(HIDAPI_LIBUSB_LINK_LIBRARIES hidapi_libusb LINK_LIBRARIES) +target_link_libraries(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_LINK_LIBRARIES}) + +add_test(NAME LibUsbReadThreadStartupTest COMMAND read_thread_startup_test) +set_tests_properties(LibUsbReadThreadStartupTest PROPERTIES TIMEOUT 30) diff --git a/libusb/test/read_thread_startup_test.c b/libusb/test/read_thread_startup_test.c new file mode 100644 index 000000000..d387687fa --- /dev/null +++ b/libusb/test/read_thread_startup_test.c @@ -0,0 +1,443 @@ +/* + * Regression test for the libusb backend read-thread startup + * (https://github.com/libusb/hidapi/issues/842). + * + * hid_open_path() starts a read thread and then waits on a two-party startup + * barrier. If the thread cannot be created, nothing ever releases that + * barrier, so the open must fail instead of waiting. + * + * The test compiles the real libusb/hid.c (and through it the real + * hidapi_thread_pthread.h) into this translation unit, and interposes: + * - pthread_create / pthread_join / pthread_barrier_wait, to inject a thread + * creation failure and observe the startup synchronization; + * - the few libusb calls hid_open_path() makes against a device, to present + * a single fake HID interface without any hardware. + * Everything else is the unmodified library code. + */ + +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + +#include +#include +#include +#include +#include +#include +#include + +#include + +/* ---- Interposed pthread functions ----------------------------------------- */ + +static pthread_mutex_t counters_mutex = PTHREAD_MUTEX_INITIALIZER; +static int inject_create_failure = 0; +static int create_failed = 0; +static int create_calls = 0; +static int join_calls = 0; +static int barrier_calls = 0; +/* The hid_device passed to the read thread, i.e. the device being opened. */ +static void *opened_dev = NULL; + +static void fatal(const char *message) +{ + fprintf(stderr, "FAIL: %s\n", message); + fflush(stderr); + /* Don't run any further library code (or atexit handlers) from here. */ + _exit(1); +} + +static int count_call(int *counter) +{ + int value; + pthread_mutex_lock(&counters_mutex); + value = ++*counter; + pthread_mutex_unlock(&counters_mutex); + return value; +} + +static int read_counter(const int *counter) +{ + int value; + pthread_mutex_lock(&counters_mutex); + value = *counter; + pthread_mutex_unlock(&counters_mutex); + return value; +} + +static int test_pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*func)(void *), void *arg) +{ + count_call(&create_calls); + opened_dev = arg; + if (inject_create_failure) { + create_failed = 1; + return EAGAIN; + } + return pthread_create(thread, attr, func, arg); +} + +static int test_pthread_join(pthread_t thread, void **retval) +{ + if (create_failed) + fatal("joined a read thread that was never created"); + count_call(&join_calls); + return pthread_join(thread, retval); +} + +static int test_pthread_barrier_wait(pthread_barrier_t *barrier) +{ + /* With no read thread, the real barrier would never be released. */ + if (create_failed) + fatal("waited on the startup barrier after the read thread failed to start"); + count_call(&barrier_calls); + return pthread_barrier_wait(barrier); +} + +/* ---- Fake libusb device: one HID interface at path "1-1:1.0" -------------- */ + +#define TEST_INTERFACE_NUMBER 0 +#define TEST_DEVICE_PATH "1-1:1.0" + +static char fake_context_storage; +static char fake_device_storage; +static char fake_handle_storage; +#define FAKE_CONTEXT ((libusb_context *)&fake_context_storage) +#define FAKE_DEVICE ((libusb_device *)&fake_device_storage) +#define FAKE_HANDLE ((libusb_device_handle *)&fake_handle_storage) + +static libusb_device *fake_device_list[2]; +static struct libusb_interface_descriptor fake_intf_desc; +static struct libusb_interface fake_interface; +static struct libusb_config_descriptor fake_config; + +static int open_calls, close_calls; +static int claim_calls, release_calls; +static int detach_calls, attach_calls; +static int ring_destroyed_before_close = -1; + +static void setup_fake_device(void) +{ + memset(&fake_intf_desc, 0, sizeof(fake_intf_desc)); + fake_intf_desc.bLength = LIBUSB_DT_INTERFACE_SIZE; + fake_intf_desc.bDescriptorType = LIBUSB_DT_INTERFACE; + fake_intf_desc.bInterfaceNumber = TEST_INTERFACE_NUMBER; + fake_intf_desc.bInterfaceClass = LIBUSB_CLASS_HID; + /* No interrupt IN endpoint: the read thread passes the startup barrier + and exits, without needing libusb event handling. */ + fake_intf_desc.bNumEndpoints = 0; + + memset(&fake_interface, 0, sizeof(fake_interface)); + fake_interface.altsetting = &fake_intf_desc; + fake_interface.num_altsetting = 1; + + memset(&fake_config, 0, sizeof(fake_config)); + fake_config.bLength = LIBUSB_DT_CONFIG_SIZE; + fake_config.bDescriptorType = LIBUSB_DT_CONFIG; + fake_config.bNumInterfaces = 1; + fake_config.bConfigurationValue = 1; + fake_config.interface = &fake_interface; + + fake_device_list[0] = FAKE_DEVICE; + fake_device_list[1] = NULL; +} + +static int test_libusb_init(libusb_context **ctx) +{ + *ctx = FAKE_CONTEXT; + return LIBUSB_SUCCESS; +} + +static void test_libusb_exit(libusb_context *ctx) +{ + (void)ctx; +} + +static ssize_t test_libusb_get_device_list(libusb_context *ctx, libusb_device ***list) +{ + (void)ctx; + *list = fake_device_list; + return 1; +} + +static void test_libusb_free_device_list(libusb_device **list, int unref_devices) +{ + (void)list; + (void)unref_devices; +} + +static int test_libusb_get_device_descriptor(libusb_device *dev, struct libusb_device_descriptor *desc) +{ + (void)dev; + memset(desc, 0, sizeof(*desc)); + desc->bLength = LIBUSB_DT_DEVICE_SIZE; + desc->bDescriptorType = LIBUSB_DT_DEVICE; + desc->idVendor = 0x1234; + desc->idProduct = 0x5678; + desc->bNumConfigurations = 1; + return LIBUSB_SUCCESS; +} + +static int test_libusb_get_active_config_descriptor(libusb_device *dev, struct libusb_config_descriptor **config) +{ + (void)dev; + *config = &fake_config; + return LIBUSB_SUCCESS; +} + +static int test_libusb_get_config_descriptor(libusb_device *dev, uint8_t config_index, struct libusb_config_descriptor **config) +{ + (void)config_index; + return test_libusb_get_active_config_descriptor(dev, config); +} + +static void test_libusb_free_config_descriptor(struct libusb_config_descriptor *config) +{ + (void)config; +} + +static int test_libusb_get_port_numbers(libusb_device *dev, uint8_t *port_numbers, int port_numbers_len) +{ + (void)dev; + if (port_numbers_len < 1) + return LIBUSB_ERROR_OVERFLOW; + port_numbers[0] = 1; + return 1; +} + +static uint8_t test_libusb_get_bus_number(libusb_device *dev) +{ + (void)dev; + return 1; +} + +static libusb_device *test_libusb_get_device(libusb_device_handle *dev_handle) +{ + (void)dev_handle; + return FAKE_DEVICE; +} + +static int test_libusb_open(libusb_device *dev, libusb_device_handle **dev_handle) +{ + (void)dev; + count_call(&open_calls); + *dev_handle = FAKE_HANDLE; + return LIBUSB_SUCCESS; +} + +static void test_libusb_close(libusb_device_handle *dev_handle); + +static int test_libusb_kernel_driver_active(libusb_device_handle *dev_handle, int interface_number) +{ + (void)dev_handle; + (void)interface_number; + /* Pretend a kernel driver is bound, so the open detaches it. */ + return 1; +} + +static int test_libusb_detach_kernel_driver(libusb_device_handle *dev_handle, int interface_number) +{ + (void)dev_handle; + if (interface_number != TEST_INTERFACE_NUMBER) + fatal("detached the kernel driver from an unexpected interface"); + count_call(&detach_calls); + return LIBUSB_SUCCESS; +} + +static int test_libusb_attach_kernel_driver(libusb_device_handle *dev_handle, int interface_number) +{ + (void)dev_handle; + if (interface_number != TEST_INTERFACE_NUMBER) + fatal("reattached the kernel driver to an unexpected interface"); + count_call(&attach_calls); + return LIBUSB_SUCCESS; +} + +static int test_libusb_claim_interface(libusb_device_handle *dev_handle, int interface_number) +{ + (void)dev_handle; + if (interface_number != TEST_INTERFACE_NUMBER) + fatal("claimed an unexpected interface"); + count_call(&claim_calls); + return LIBUSB_SUCCESS; +} + +static int test_libusb_release_interface(libusb_device_handle *dev_handle, int interface_number) +{ + (void)dev_handle; + if (interface_number != TEST_INTERFACE_NUMBER) + fatal("released an unexpected interface"); + count_call(&release_calls); + return LIBUSB_SUCCESS; +} + +static int test_libusb_control_transfer(libusb_device_handle *dev_handle, uint8_t request_type, uint8_t bRequest, uint16_t wValue, uint16_t wIndex, unsigned char *data, uint16_t wLength, unsigned int timeout) +{ + (void)dev_handle; (void)request_type; (void)bRequest; (void)wValue; + (void)wIndex; (void)data; (void)wLength; (void)timeout; + /* No report descriptor; the open doesn't depend on it. */ + return LIBUSB_ERROR_PIPE; +} + +/* ---- The library under test ----------------------------------------------- */ + +/* and are already included (and include-guarded), so + these only rename the calls made by the library code below. */ +#define pthread_create test_pthread_create +#define pthread_join test_pthread_join +#define pthread_barrier_wait test_pthread_barrier_wait + +#define libusb_init test_libusb_init +#define libusb_exit test_libusb_exit +#define libusb_get_device_list test_libusb_get_device_list +#define libusb_free_device_list test_libusb_free_device_list +#define libusb_get_device_descriptor test_libusb_get_device_descriptor +#define libusb_get_active_config_descriptor test_libusb_get_active_config_descriptor +#define libusb_get_config_descriptor test_libusb_get_config_descriptor +#define libusb_free_config_descriptor test_libusb_free_config_descriptor +#define libusb_get_port_numbers test_libusb_get_port_numbers +#define libusb_get_bus_number test_libusb_get_bus_number +#define libusb_get_device test_libusb_get_device +#define libusb_open test_libusb_open +#define libusb_close test_libusb_close +#define libusb_kernel_driver_active test_libusb_kernel_driver_active +#define libusb_detach_kernel_driver test_libusb_detach_kernel_driver +#define libusb_attach_kernel_driver test_libusb_attach_kernel_driver +#define libusb_claim_interface test_libusb_claim_interface +#define libusb_release_interface test_libusb_release_interface +#define libusb_control_transfer test_libusb_control_transfer + +#include "../hid.c" + +#undef pthread_create +#undef pthread_join +#undef pthread_barrier_wait + +static void test_libusb_close(libusb_device_handle *dev_handle) +{ + (void)dev_handle; + count_call(&close_calls); + /* Both hid_open_path() (on failure) and hid_close() close the handle + before freeing the device: its input ring must already be gone in the + former case, and still be allocated in the latter. */ + if (opened_dev) + ring_destroyed_before_close = ((hid_device *)opened_dev)->input_ring.storage == NULL; +} + +/* ---- Test driver ------------------------------------------------------------ */ + +#ifdef DETACH_KERNEL_DRIVER +#define EXPECTED_DRIVER_CALLS 1 +#else +#define EXPECTED_DRIVER_CALLS 0 +#endif + +static int failures = 0; + +#define CHECK(cond) do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL (%s:%d): %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ +} while (0) + +static void reset_counters(void) +{ + pthread_mutex_lock(&counters_mutex); + create_failed = 0; + create_calls = join_calls = barrier_calls = 0; + open_calls = close_calls = 0; + claim_calls = release_calls = 0; + detach_calls = attach_calls = 0; + ring_destroyed_before_close = -1; + opened_dev = NULL; + pthread_mutex_unlock(&counters_mutex); +} + +static void test_thread_create_failure(void) +{ + hid_device *dev; + const wchar_t *err; + + reset_counters(); + inject_create_failure = 1; + + /* Must return instead of blocking in the startup barrier. */ + dev = hid_open_path(TEST_DEVICE_PATH); + inject_create_failure = 0; + + CHECK(dev == NULL); + CHECK(read_counter(&create_calls) == 1); + CHECK(read_counter(&barrier_calls) == 0); + CHECK(read_counter(&join_calls) == 0); + + /* Everything acquired for the open is given back. */ + CHECK(read_counter(&open_calls) == 1); + CHECK(read_counter(&close_calls) == 1); + CHECK(read_counter(&claim_calls) == 1); + CHECK(read_counter(&release_calls) == 1); + CHECK(read_counter(&detach_calls) == EXPECTED_DRIVER_CALLS); + CHECK(read_counter(&attach_calls) == EXPECTED_DRIVER_CALLS); + CHECK(ring_destroyed_before_close == 1); + + /* The failure is reported. */ + err = hid_error(NULL); + CHECK(err != NULL && wcsstr(err, L"failed to initialize device") != NULL); + if (dev) + hid_close(dev); +} + +static void test_normal_startup(void) +{ + hid_device *dev; + + reset_counters(); + + dev = hid_open_path(TEST_DEVICE_PATH); + CHECK(dev != NULL); + if (!dev) + return; + + CHECK(read_counter(&create_calls) == 1); + /* Both the opening thread and the read thread passed the barrier. */ + CHECK(read_counter(&barrier_calls) == 2); + CHECK(read_counter(&join_calls) == 0); + CHECK(read_counter(&claim_calls) == 1); + CHECK(read_counter(&release_calls) == 0); + CHECK(read_counter(&detach_calls) == EXPECTED_DRIVER_CALLS); + CHECK(read_counter(&attach_calls) == 0); + CHECK(read_counter(&close_calls) == 0); + CHECK(dev->input_ring.storage != NULL); + CHECK(wcscmp(hid_error(NULL), L"Success") == 0); + + hid_close(dev); + + CHECK(read_counter(&join_calls) == 1); + CHECK(read_counter(&release_calls) == 1); + CHECK(read_counter(&attach_calls) == EXPECTED_DRIVER_CALLS); + CHECK(read_counter(&close_calls) == 1); + CHECK(ring_destroyed_before_close == 0); +} + +int main(void) +{ + /* Watchdog: a regression must fail the test rather than hang it. */ + alarm(10); + + setup_fake_device(); + + CHECK(hid_init() == 0); + + test_thread_create_failure(); + /* The library stays usable after a failed open. */ + test_normal_startup(); + + CHECK(hid_exit() == 0); + + if (failures) { + fprintf(stderr, "%d check(s) failed\n", failures); + return 1; + } + printf("PASS\n"); + return 0; +} From a32f58e898f2288278fa2263bcb3a81884c419e8 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Sun, 8 Nov 2026 21:41:05 +0200 Subject: [PATCH 2/3] libusb: only build the read-thread startup test with native barriers On Android before API 24, hidapi_thread_pthread.h provides its own pthread_barrier_t and pthread_barrier_wait(), which the test's interposition of pthread_barrier_wait() collides with. Add the test only where a native pthread_barrier_wait() exists (checked with the thread library linked), and make the test say so explicitly if it is built elsewhere. The test also now always uses the bundled pthread thread model, even if a custom HIDAPI_THREAD_MODEL_INCLUDE is configured for the library. Reword the hidapi_thread_create() comment: pthread_create() doesn't guarantee the thread handle is left unchanged on failure. Assisted-by: claude-code:claude-opus-5-5 --- libusb/hidapi_thread_pthread.h | 4 +-- libusb/test/CMakeLists.txt | 38 +++++++++++++++++--------- libusb/test/read_thread_startup_test.c | 11 ++++++++ 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/libusb/hidapi_thread_pthread.h b/libusb/hidapi_thread_pthread.h index 52d3cc68f..950df0544 100644 --- a/libusb/hidapi_thread_pthread.h +++ b/libusb/hidapi_thread_pthread.h @@ -149,8 +149,8 @@ static void hidapi_thread_barrier_wait(hidapi_thread_state *state) } /* Starts `func` on a new thread. Returns 0 on success, or a non-zero value when - the thread could not be started (in which case `state->thread` is left unset - and must not be joined). */ + the thread could not be started (in which case no thread was started and + `state->thread` must not be joined). */ static int hidapi_thread_create(hidapi_thread_state *state, void *(*func)(void*), void *func_arg) { return pthread_create(&state->thread, NULL, func, func_arg); diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index f946819be..576a53348 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -62,17 +62,29 @@ endforeach() # Read-thread startup: libusb/hid.c is compiled into the test, with pthread and # device-level libusb calls interposed, so no hardware is needed. Built with the # same definitions and libraries as the hidapi_libusb library itself. -add_executable(read_thread_startup_test read_thread_startup_test.c) -hidapi_configure_sanitizers(read_thread_startup_test) -if(HIDAPI_BUILD_AS_CXX) - set_source_files_properties(read_thread_startup_test.c PROPERTIES LANGUAGE CXX) -endif() -get_target_property(HIDAPI_LIBUSB_DEFINITIONS hidapi_libusb COMPILE_DEFINITIONS) -if(HIDAPI_LIBUSB_DEFINITIONS) - target_compile_definitions(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_DEFINITIONS}) -endif() -get_target_property(HIDAPI_LIBUSB_LINK_LIBRARIES hidapi_libusb LINK_LIBRARIES) -target_link_libraries(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_LINK_LIBRARIES}) +# It interposes the native pthread_barrier_wait(), so it is only added where that +# exists (not on Android before API 24, where hidapi_thread_pthread.h provides +# its own barrier implementation). +include(CheckSymbolExists) +include(CMakePushCheckState) +cmake_push_check_state(RESET) +set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_THREAD_LIBS_INIT}) +check_symbol_exists(pthread_barrier_wait "pthread.h" HIDAPI_HAVE_PTHREAD_BARRIER_WAIT) +cmake_pop_check_state() + +if(HIDAPI_HAVE_PTHREAD_BARRIER_WAIT) + add_executable(read_thread_startup_test read_thread_startup_test.c) + hidapi_configure_sanitizers(read_thread_startup_test) + if(HIDAPI_BUILD_AS_CXX) + set_source_files_properties(read_thread_startup_test.c PROPERTIES LANGUAGE CXX) + endif() + get_target_property(HIDAPI_LIBUSB_DEFINITIONS hidapi_libusb COMPILE_DEFINITIONS) + if(HIDAPI_LIBUSB_DEFINITIONS) + target_compile_definitions(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_DEFINITIONS}) + endif() + get_target_property(HIDAPI_LIBUSB_LINK_LIBRARIES hidapi_libusb LINK_LIBRARIES) + target_link_libraries(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_LINK_LIBRARIES}) -add_test(NAME LibUsbReadThreadStartupTest COMMAND read_thread_startup_test) -set_tests_properties(LibUsbReadThreadStartupTest PROPERTIES TIMEOUT 30) + add_test(NAME LibUsbReadThreadStartupTest COMMAND read_thread_startup_test) + set_tests_properties(LibUsbReadThreadStartupTest PROPERTIES TIMEOUT 30) +endif() diff --git a/libusb/test/read_thread_startup_test.c b/libusb/test/read_thread_startup_test.c index d387687fa..20ec1de6f 100644 --- a/libusb/test/read_thread_startup_test.c +++ b/libusb/test/read_thread_startup_test.c @@ -13,6 +13,10 @@ * - the few libusb calls hid_open_path() makes against a device, to present * a single fake HID interface without any hardware. * Everything else is the unmodified library code. + * + * It always tests the bundled pthread thread model, even if the library is + * configured with a custom HIDAPI_THREAD_MODEL_INCLUDE, and needs a native + * pthread_barrier_wait() (see CMakeLists.txt). */ #ifndef _GNU_SOURCE @@ -29,6 +33,10 @@ #include +#if defined(__ANDROID__) && __ANDROID_API__ < __ANDROID_API_N__ +#error "needs a native pthread_barrier_wait(); hidapi_thread_pthread.h provides its own here" +#endif + /* ---- Interposed pthread functions ----------------------------------------- */ static pthread_mutex_t counters_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -307,6 +315,9 @@ static int test_libusb_control_transfer(libusb_device_handle *dev_handle, uint8_ #define libusb_release_interface test_libusb_release_interface #define libusb_control_transfer test_libusb_control_transfer +/* The interposition above targets the bundled pthread thread model. */ +#undef HIDAPI_THREAD_MODEL_INCLUDE + #include "../hid.c" #undef pthread_create From 55d24be2f3e8e8e1fce57e5de2feeae2ae2cb19d Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Mon, 9 Nov 2026 00:11:29 +0200 Subject: [PATCH 3/3] Keep the thread-startup fix focused on error handling Remove the regression test and restore the test CMake file to the base version, leaving only the production thread-creation failure handling. Assisted-by: Codex:GPT-6 --- libusb/test/CMakeLists.txt | 30 -- libusb/test/read_thread_startup_test.c | 454 ------------------------- 2 files changed, 484 deletions(-) delete mode 100644 libusb/test/read_thread_startup_test.c diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index 576a53348..ca5d4a914 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -58,33 +58,3 @@ foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES}) WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" ) endforeach() - -# Read-thread startup: libusb/hid.c is compiled into the test, with pthread and -# device-level libusb calls interposed, so no hardware is needed. Built with the -# same definitions and libraries as the hidapi_libusb library itself. -# It interposes the native pthread_barrier_wait(), so it is only added where that -# exists (not on Android before API 24, where hidapi_thread_pthread.h provides -# its own barrier implementation). -include(CheckSymbolExists) -include(CMakePushCheckState) -cmake_push_check_state(RESET) -set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_THREAD_LIBS_INIT}) -check_symbol_exists(pthread_barrier_wait "pthread.h" HIDAPI_HAVE_PTHREAD_BARRIER_WAIT) -cmake_pop_check_state() - -if(HIDAPI_HAVE_PTHREAD_BARRIER_WAIT) - add_executable(read_thread_startup_test read_thread_startup_test.c) - hidapi_configure_sanitizers(read_thread_startup_test) - if(HIDAPI_BUILD_AS_CXX) - set_source_files_properties(read_thread_startup_test.c PROPERTIES LANGUAGE CXX) - endif() - get_target_property(HIDAPI_LIBUSB_DEFINITIONS hidapi_libusb COMPILE_DEFINITIONS) - if(HIDAPI_LIBUSB_DEFINITIONS) - target_compile_definitions(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_DEFINITIONS}) - endif() - get_target_property(HIDAPI_LIBUSB_LINK_LIBRARIES hidapi_libusb LINK_LIBRARIES) - target_link_libraries(read_thread_startup_test PRIVATE ${HIDAPI_LIBUSB_LINK_LIBRARIES}) - - add_test(NAME LibUsbReadThreadStartupTest COMMAND read_thread_startup_test) - set_tests_properties(LibUsbReadThreadStartupTest PROPERTIES TIMEOUT 30) -endif() diff --git a/libusb/test/read_thread_startup_test.c b/libusb/test/read_thread_startup_test.c deleted file mode 100644 index 20ec1de6f..000000000 --- a/libusb/test/read_thread_startup_test.c +++ /dev/null @@ -1,454 +0,0 @@ -/* - * Regression test for the libusb backend read-thread startup - * (https://github.com/libusb/hidapi/issues/842). - * - * hid_open_path() starts a read thread and then waits on a two-party startup - * barrier. If the thread cannot be created, nothing ever releases that - * barrier, so the open must fail instead of waiting. - * - * The test compiles the real libusb/hid.c (and through it the real - * hidapi_thread_pthread.h) into this translation unit, and interposes: - * - pthread_create / pthread_join / pthread_barrier_wait, to inject a thread - * creation failure and observe the startup synchronization; - * - the few libusb calls hid_open_path() makes against a device, to present - * a single fake HID interface without any hardware. - * Everything else is the unmodified library code. - * - * It always tests the bundled pthread thread model, even if the library is - * configured with a custom HIDAPI_THREAD_MODEL_INCLUDE, and needs a native - * pthread_barrier_wait() (see CMakeLists.txt). - */ - -#ifndef _GNU_SOURCE -#define _GNU_SOURCE -#endif - -#include -#include -#include -#include -#include -#include -#include - -#include - -#if defined(__ANDROID__) && __ANDROID_API__ < __ANDROID_API_N__ -#error "needs a native pthread_barrier_wait(); hidapi_thread_pthread.h provides its own here" -#endif - -/* ---- Interposed pthread functions ----------------------------------------- */ - -static pthread_mutex_t counters_mutex = PTHREAD_MUTEX_INITIALIZER; -static int inject_create_failure = 0; -static int create_failed = 0; -static int create_calls = 0; -static int join_calls = 0; -static int barrier_calls = 0; -/* The hid_device passed to the read thread, i.e. the device being opened. */ -static void *opened_dev = NULL; - -static void fatal(const char *message) -{ - fprintf(stderr, "FAIL: %s\n", message); - fflush(stderr); - /* Don't run any further library code (or atexit handlers) from here. */ - _exit(1); -} - -static int count_call(int *counter) -{ - int value; - pthread_mutex_lock(&counters_mutex); - value = ++*counter; - pthread_mutex_unlock(&counters_mutex); - return value; -} - -static int read_counter(const int *counter) -{ - int value; - pthread_mutex_lock(&counters_mutex); - value = *counter; - pthread_mutex_unlock(&counters_mutex); - return value; -} - -static int test_pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*func)(void *), void *arg) -{ - count_call(&create_calls); - opened_dev = arg; - if (inject_create_failure) { - create_failed = 1; - return EAGAIN; - } - return pthread_create(thread, attr, func, arg); -} - -static int test_pthread_join(pthread_t thread, void **retval) -{ - if (create_failed) - fatal("joined a read thread that was never created"); - count_call(&join_calls); - return pthread_join(thread, retval); -} - -static int test_pthread_barrier_wait(pthread_barrier_t *barrier) -{ - /* With no read thread, the real barrier would never be released. */ - if (create_failed) - fatal("waited on the startup barrier after the read thread failed to start"); - count_call(&barrier_calls); - return pthread_barrier_wait(barrier); -} - -/* ---- Fake libusb device: one HID interface at path "1-1:1.0" -------------- */ - -#define TEST_INTERFACE_NUMBER 0 -#define TEST_DEVICE_PATH "1-1:1.0" - -static char fake_context_storage; -static char fake_device_storage; -static char fake_handle_storage; -#define FAKE_CONTEXT ((libusb_context *)&fake_context_storage) -#define FAKE_DEVICE ((libusb_device *)&fake_device_storage) -#define FAKE_HANDLE ((libusb_device_handle *)&fake_handle_storage) - -static libusb_device *fake_device_list[2]; -static struct libusb_interface_descriptor fake_intf_desc; -static struct libusb_interface fake_interface; -static struct libusb_config_descriptor fake_config; - -static int open_calls, close_calls; -static int claim_calls, release_calls; -static int detach_calls, attach_calls; -static int ring_destroyed_before_close = -1; - -static void setup_fake_device(void) -{ - memset(&fake_intf_desc, 0, sizeof(fake_intf_desc)); - fake_intf_desc.bLength = LIBUSB_DT_INTERFACE_SIZE; - fake_intf_desc.bDescriptorType = LIBUSB_DT_INTERFACE; - fake_intf_desc.bInterfaceNumber = TEST_INTERFACE_NUMBER; - fake_intf_desc.bInterfaceClass = LIBUSB_CLASS_HID; - /* No interrupt IN endpoint: the read thread passes the startup barrier - and exits, without needing libusb event handling. */ - fake_intf_desc.bNumEndpoints = 0; - - memset(&fake_interface, 0, sizeof(fake_interface)); - fake_interface.altsetting = &fake_intf_desc; - fake_interface.num_altsetting = 1; - - memset(&fake_config, 0, sizeof(fake_config)); - fake_config.bLength = LIBUSB_DT_CONFIG_SIZE; - fake_config.bDescriptorType = LIBUSB_DT_CONFIG; - fake_config.bNumInterfaces = 1; - fake_config.bConfigurationValue = 1; - fake_config.interface = &fake_interface; - - fake_device_list[0] = FAKE_DEVICE; - fake_device_list[1] = NULL; -} - -static int test_libusb_init(libusb_context **ctx) -{ - *ctx = FAKE_CONTEXT; - return LIBUSB_SUCCESS; -} - -static void test_libusb_exit(libusb_context *ctx) -{ - (void)ctx; -} - -static ssize_t test_libusb_get_device_list(libusb_context *ctx, libusb_device ***list) -{ - (void)ctx; - *list = fake_device_list; - return 1; -} - -static void test_libusb_free_device_list(libusb_device **list, int unref_devices) -{ - (void)list; - (void)unref_devices; -} - -static int test_libusb_get_device_descriptor(libusb_device *dev, struct libusb_device_descriptor *desc) -{ - (void)dev; - memset(desc, 0, sizeof(*desc)); - desc->bLength = LIBUSB_DT_DEVICE_SIZE; - desc->bDescriptorType = LIBUSB_DT_DEVICE; - desc->idVendor = 0x1234; - desc->idProduct = 0x5678; - desc->bNumConfigurations = 1; - return LIBUSB_SUCCESS; -} - -static int test_libusb_get_active_config_descriptor(libusb_device *dev, struct libusb_config_descriptor **config) -{ - (void)dev; - *config = &fake_config; - return LIBUSB_SUCCESS; -} - -static int test_libusb_get_config_descriptor(libusb_device *dev, uint8_t config_index, struct libusb_config_descriptor **config) -{ - (void)config_index; - return test_libusb_get_active_config_descriptor(dev, config); -} - -static void test_libusb_free_config_descriptor(struct libusb_config_descriptor *config) -{ - (void)config; -} - -static int test_libusb_get_port_numbers(libusb_device *dev, uint8_t *port_numbers, int port_numbers_len) -{ - (void)dev; - if (port_numbers_len < 1) - return LIBUSB_ERROR_OVERFLOW; - port_numbers[0] = 1; - return 1; -} - -static uint8_t test_libusb_get_bus_number(libusb_device *dev) -{ - (void)dev; - return 1; -} - -static libusb_device *test_libusb_get_device(libusb_device_handle *dev_handle) -{ - (void)dev_handle; - return FAKE_DEVICE; -} - -static int test_libusb_open(libusb_device *dev, libusb_device_handle **dev_handle) -{ - (void)dev; - count_call(&open_calls); - *dev_handle = FAKE_HANDLE; - return LIBUSB_SUCCESS; -} - -static void test_libusb_close(libusb_device_handle *dev_handle); - -static int test_libusb_kernel_driver_active(libusb_device_handle *dev_handle, int interface_number) -{ - (void)dev_handle; - (void)interface_number; - /* Pretend a kernel driver is bound, so the open detaches it. */ - return 1; -} - -static int test_libusb_detach_kernel_driver(libusb_device_handle *dev_handle, int interface_number) -{ - (void)dev_handle; - if (interface_number != TEST_INTERFACE_NUMBER) - fatal("detached the kernel driver from an unexpected interface"); - count_call(&detach_calls); - return LIBUSB_SUCCESS; -} - -static int test_libusb_attach_kernel_driver(libusb_device_handle *dev_handle, int interface_number) -{ - (void)dev_handle; - if (interface_number != TEST_INTERFACE_NUMBER) - fatal("reattached the kernel driver to an unexpected interface"); - count_call(&attach_calls); - return LIBUSB_SUCCESS; -} - -static int test_libusb_claim_interface(libusb_device_handle *dev_handle, int interface_number) -{ - (void)dev_handle; - if (interface_number != TEST_INTERFACE_NUMBER) - fatal("claimed an unexpected interface"); - count_call(&claim_calls); - return LIBUSB_SUCCESS; -} - -static int test_libusb_release_interface(libusb_device_handle *dev_handle, int interface_number) -{ - (void)dev_handle; - if (interface_number != TEST_INTERFACE_NUMBER) - fatal("released an unexpected interface"); - count_call(&release_calls); - return LIBUSB_SUCCESS; -} - -static int test_libusb_control_transfer(libusb_device_handle *dev_handle, uint8_t request_type, uint8_t bRequest, uint16_t wValue, uint16_t wIndex, unsigned char *data, uint16_t wLength, unsigned int timeout) -{ - (void)dev_handle; (void)request_type; (void)bRequest; (void)wValue; - (void)wIndex; (void)data; (void)wLength; (void)timeout; - /* No report descriptor; the open doesn't depend on it. */ - return LIBUSB_ERROR_PIPE; -} - -/* ---- The library under test ----------------------------------------------- */ - -/* and are already included (and include-guarded), so - these only rename the calls made by the library code below. */ -#define pthread_create test_pthread_create -#define pthread_join test_pthread_join -#define pthread_barrier_wait test_pthread_barrier_wait - -#define libusb_init test_libusb_init -#define libusb_exit test_libusb_exit -#define libusb_get_device_list test_libusb_get_device_list -#define libusb_free_device_list test_libusb_free_device_list -#define libusb_get_device_descriptor test_libusb_get_device_descriptor -#define libusb_get_active_config_descriptor test_libusb_get_active_config_descriptor -#define libusb_get_config_descriptor test_libusb_get_config_descriptor -#define libusb_free_config_descriptor test_libusb_free_config_descriptor -#define libusb_get_port_numbers test_libusb_get_port_numbers -#define libusb_get_bus_number test_libusb_get_bus_number -#define libusb_get_device test_libusb_get_device -#define libusb_open test_libusb_open -#define libusb_close test_libusb_close -#define libusb_kernel_driver_active test_libusb_kernel_driver_active -#define libusb_detach_kernel_driver test_libusb_detach_kernel_driver -#define libusb_attach_kernel_driver test_libusb_attach_kernel_driver -#define libusb_claim_interface test_libusb_claim_interface -#define libusb_release_interface test_libusb_release_interface -#define libusb_control_transfer test_libusb_control_transfer - -/* The interposition above targets the bundled pthread thread model. */ -#undef HIDAPI_THREAD_MODEL_INCLUDE - -#include "../hid.c" - -#undef pthread_create -#undef pthread_join -#undef pthread_barrier_wait - -static void test_libusb_close(libusb_device_handle *dev_handle) -{ - (void)dev_handle; - count_call(&close_calls); - /* Both hid_open_path() (on failure) and hid_close() close the handle - before freeing the device: its input ring must already be gone in the - former case, and still be allocated in the latter. */ - if (opened_dev) - ring_destroyed_before_close = ((hid_device *)opened_dev)->input_ring.storage == NULL; -} - -/* ---- Test driver ------------------------------------------------------------ */ - -#ifdef DETACH_KERNEL_DRIVER -#define EXPECTED_DRIVER_CALLS 1 -#else -#define EXPECTED_DRIVER_CALLS 0 -#endif - -static int failures = 0; - -#define CHECK(cond) do { \ - if (!(cond)) { \ - fprintf(stderr, "FAIL (%s:%d): %s\n", __FILE__, __LINE__, #cond); \ - failures++; \ - } \ -} while (0) - -static void reset_counters(void) -{ - pthread_mutex_lock(&counters_mutex); - create_failed = 0; - create_calls = join_calls = barrier_calls = 0; - open_calls = close_calls = 0; - claim_calls = release_calls = 0; - detach_calls = attach_calls = 0; - ring_destroyed_before_close = -1; - opened_dev = NULL; - pthread_mutex_unlock(&counters_mutex); -} - -static void test_thread_create_failure(void) -{ - hid_device *dev; - const wchar_t *err; - - reset_counters(); - inject_create_failure = 1; - - /* Must return instead of blocking in the startup barrier. */ - dev = hid_open_path(TEST_DEVICE_PATH); - inject_create_failure = 0; - - CHECK(dev == NULL); - CHECK(read_counter(&create_calls) == 1); - CHECK(read_counter(&barrier_calls) == 0); - CHECK(read_counter(&join_calls) == 0); - - /* Everything acquired for the open is given back. */ - CHECK(read_counter(&open_calls) == 1); - CHECK(read_counter(&close_calls) == 1); - CHECK(read_counter(&claim_calls) == 1); - CHECK(read_counter(&release_calls) == 1); - CHECK(read_counter(&detach_calls) == EXPECTED_DRIVER_CALLS); - CHECK(read_counter(&attach_calls) == EXPECTED_DRIVER_CALLS); - CHECK(ring_destroyed_before_close == 1); - - /* The failure is reported. */ - err = hid_error(NULL); - CHECK(err != NULL && wcsstr(err, L"failed to initialize device") != NULL); - if (dev) - hid_close(dev); -} - -static void test_normal_startup(void) -{ - hid_device *dev; - - reset_counters(); - - dev = hid_open_path(TEST_DEVICE_PATH); - CHECK(dev != NULL); - if (!dev) - return; - - CHECK(read_counter(&create_calls) == 1); - /* Both the opening thread and the read thread passed the barrier. */ - CHECK(read_counter(&barrier_calls) == 2); - CHECK(read_counter(&join_calls) == 0); - CHECK(read_counter(&claim_calls) == 1); - CHECK(read_counter(&release_calls) == 0); - CHECK(read_counter(&detach_calls) == EXPECTED_DRIVER_CALLS); - CHECK(read_counter(&attach_calls) == 0); - CHECK(read_counter(&close_calls) == 0); - CHECK(dev->input_ring.storage != NULL); - CHECK(wcscmp(hid_error(NULL), L"Success") == 0); - - hid_close(dev); - - CHECK(read_counter(&join_calls) == 1); - CHECK(read_counter(&release_calls) == 1); - CHECK(read_counter(&attach_calls) == EXPECTED_DRIVER_CALLS); - CHECK(read_counter(&close_calls) == 1); - CHECK(ring_destroyed_before_close == 0); -} - -int main(void) -{ - /* Watchdog: a regression must fail the test rather than hang it. */ - alarm(10); - - setup_fake_device(); - - CHECK(hid_init() == 0); - - test_thread_create_failure(); - /* The library stays usable after a failed open. */ - test_normal_startup(); - - CHECK(hid_exit() == 0); - - if (failures) { - fprintf(stderr, "%d check(s) failed\n", failures); - return 1; - } - printf("PASS\n"); - return 0; -}