From f0a2db5750c9aaabea22b71e9e3ae7d63a4ad656 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Mon, 27 Apr 2026 00:50:56 +0300 Subject: [PATCH 1/2] Add hid_read_interrupt API for thread-safe read cancellation New functions on hid_device, implemented across all five backends (linux, libusb, mac, windows, netbsd): hid_read_interrupt - asynchronously cancel a blocked read hid_is_read_interrupted - query the sticky interrupt state hid_read_clear_interrupt - clear the state, allowing reads to resume hid_read_interrupt is the only hidapi function safe to call concurrently with another function on the same device. Other device operations (write, feature/output reports, info getters) are unaffected. Allows a dedicated reader thread to be stopped without busy-polling on a short timeout. New hid_read_test tool, to check new functionality. Resolves: #146 Assisted-by: Claude:claude-opus-4.7 --- CMakeLists.txt | 5 ++ hid_read_test/CMakeLists.txt | 42 ++++++++++ hid_read_test/main.cpp | 146 +++++++++++++++++++++++++++++++++++ hidapi/hidapi.h | 84 ++++++++++++++++++++ libusb/hid.c | 46 ++++++++++- linux/hid.c | 84 ++++++++++++++++---- mac/hid.c | 45 ++++++++++- netbsd/hid.c | 82 ++++++++++++++++++-- windows/hid.c | 45 ++++++++++- 9 files changed, 547 insertions(+), 32 deletions(-) create mode 100644 hid_read_test/CMakeLists.txt create mode 100644 hid_read_test/main.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d7086813..bdbd3b79 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -92,6 +92,11 @@ if(HIDAPI_BUILD_HIDTEST) add_subdirectory(hidtest) endif() +option(HIDAPI_BUILD_HID_READ_TEST "Build hid_read_test cmd-line tool" OFF) +if(HIDAPI_BUILD_HID_READ_TEST) + add_subdirectory(hid_read_test) +endif() + if(HIDAPI_ENABLE_ASAN) if(NOT MSVC) # MSVC doesn't recognize those options, other compilers - requiring it diff --git a/hid_read_test/CMakeLists.txt b/hid_read_test/CMakeLists.txt new file mode 100644 index 00000000..32e300f4 --- /dev/null +++ b/hid_read_test/CMakeLists.txt @@ -0,0 +1,42 @@ +cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR) +project(hid_read_test CXX) + +if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + # built as a standalone project + if(POLICY CMP0074) + cmake_policy(SET CMP0074 NEW) + endif() + + find_package(hidapi 0.16 REQUIRED) + message(STATUS "Using HIDAPI: ${hidapi_VERSION}") +else() + message(STATUS "Building hid_read_test") +endif() + +find_package(Threads REQUIRED) + +set(HIDAPI_HID_READ_TEST_TARGETS) +if(NOT WIN32 AND NOT APPLE AND CMAKE_SYSTEM_NAME MATCHES "Linux") + if(TARGET hidapi::hidraw) + add_executable(hid_read_test_hidraw main.cpp) + target_compile_features(hid_read_test_hidraw PRIVATE cxx_std_11) + target_link_libraries(hid_read_test_hidraw PRIVATE hidapi::hidraw Threads::Threads) + list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test_hidraw) + endif() + if(TARGET hidapi::libusb) + add_executable(hid_read_test_libusb main.cpp) + target_compile_features(hid_read_test_libusb PRIVATE cxx_std_11) + target_compile_definitions(hid_read_test_libusb PRIVATE USING_HIDAPI_LIBUSB) + target_link_libraries(hid_read_test_libusb PRIVATE hidapi::libusb Threads::Threads) + list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test_libusb) + endif() +else() + add_executable(hid_read_test main.cpp) + target_compile_features(hid_read_test PRIVATE cxx_std_11) + target_link_libraries(hid_read_test PRIVATE hidapi::hidapi Threads::Threads) + list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test) +endif() + +install(TARGETS ${HIDAPI_HID_READ_TEST_TARGETS} + RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" +) diff --git a/hid_read_test/main.cpp b/hid_read_test/main.cpp new file mode 100644 index 00000000..454b5a3b --- /dev/null +++ b/hid_read_test/main.cpp @@ -0,0 +1,146 @@ +/******************************************************* + HIDAPI - hid_read_test + + A small C++11 cmd-line tool that opens a HID device by + VID/PID, spawns a read thread that prints input reports + as hex with timestamps, and waits for Enter or Ctrl+C + to gracefully interrupt the read and exit. + + The contents of this file may be used by anyone for any + reason without any conditions and may be used as a + starting point for your own applications which use HIDAPI. +********************************************************/ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +std::atomic g_dev{nullptr}; + +std::string timestamp_now() +{ + using namespace std::chrono; + auto now = system_clock::now(); + auto t = system_clock::to_time_t(now); + auto ms = duration_cast(now.time_since_epoch()) % 1000; + + std::tm tm_buf{}; +#ifdef _WIN32 + localtime_s(&tm_buf, &t); +#else + localtime_r(&t, &tm_buf); +#endif + + std::ostringstream ss; + ss << std::put_time(&tm_buf, "%Y-%m-%d %H:%M:%S") + << '.' << std::setfill('0') << std::setw(3) << ms.count(); + return ss.str(); +} + +extern "C" void on_signal(int) +{ + if (hid_device *d = g_dev.load(std::memory_order_acquire)) + hid_read_interrupt(d); +} + +void read_thread_fn(hid_device *dev) +{ + unsigned char buf[4096]; + const int max_retries = 3; + int errors = 0; + + for (;;) { + int n = hid_read_timeout(dev, buf, sizeof(buf), -1); + if (n < 0) { + std::cout << '[' << timestamp_now() << "] read returned -1"; + const wchar_t *err = hid_read_error(dev); + if (err) std::wcout << L": " << err; + std::cout << std::endl; + + if (hid_is_read_interrupted(dev)) + break; + + if (++errors > max_retries) { + std::cout << '[' << timestamp_now() << "] giving up after " + << max_retries << " consecutive errors" << std::endl; + break; + } + continue; + } + + errors = 0; /* reset on a successful read */ + + if (n == 0) continue; + + std::cout << '[' << timestamp_now() << "] "; + std::cout << std::hex << std::setfill('0'); + for (int i = 0; i < n; ++i) + std::cout << std::setw(2) << static_cast(buf[i]) << ' '; + std::cout << std::dec << std::endl; + } +} + +unsigned short parse_hex_u16(const char *s) +{ + return static_cast(std::strtoul(s, nullptr, 16)); +} + +} // anonymous namespace + +int main(int argc, char **argv) +{ + if (argc < 3) { + std::cerr << "Usage: " << argv[0] << " \n" + << " vid, pid: hex (e.g. 04d8 003f)\n"; + return 1; + } + + unsigned short vid = parse_hex_u16(argv[1]); + unsigned short pid = parse_hex_u16(argv[2]); + + if (hid_init() != 0) { + std::cerr << "hid_init failed\n"; + return 1; + } + + hid_device *dev = hid_open(vid, pid, nullptr); + if (!dev) { + std::cerr << "hid_open failed for VID=" << std::hex << std::setw(4) + << std::setfill('0') << vid << " PID=" << std::setw(4) << pid + << std::dec << '\n'; + hid_exit(); + return 1; + } + g_dev.store(dev, std::memory_order_release); + + std::signal(SIGINT, on_signal); +#ifdef SIGTERM + std::signal(SIGTERM, on_signal); +#endif + + std::cout << "Reading from VID=" << std::hex << std::setw(4) << std::setfill('0') + << vid << " PID=" << std::setw(4) << pid << std::dec + << " (press Enter or Ctrl+C to exit)" << std::endl; + + std::thread reader(read_thread_fn, dev); + + std::cin.get(); /* Enter, EOF, or POSIX-EINTR from a signal */ + + hid_read_interrupt(dev); /* idempotent — also safe if signal handler ran first */ + reader.join(); + + hid_close(dev); + hid_exit(); + return 0; +} diff --git a/hidapi/hidapi.h b/hidapi/hidapi.h index cbc3107d..9176b088 100644 --- a/hidapi/hidapi.h +++ b/hidapi/hidapi.h @@ -423,6 +423,90 @@ extern "C" { */ int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonblock); + /** @brief Asynchronously interrupt blocking hid_read/hid_read_timeout call. + + Since version 0.16.0, @ref HID_API_VERSION >= HID_API_MAKE_VERSION(0, 16, 0) + + Thread-safely interrupts a blocking hid_read()/hid_read_timeout() call + currently in progress (or about to start) on @p dev. The interrupted + call returns -1 immediately; hid_read_error(dev) returns a string + indicating the read was interrupted. + + Once interrupted, subsequent calls to hid_read()/ + hid_read_timeout() also return -1 immediately, until @ref + hid_read_clear_interrupt is called. + + A hid_read*() call that observes data already buffered or + queued before the interrupt may return that data; otherwise it + returns -1. Eventually (within at most one further call) hid_read*() + will return -1. + + Only the read pipeline is affected: hid_write(), hid_get_input_report(), + feature/output report functions, and all other operations on @p dev + continue to work normally. + + This is the only function on hid_device that may be called concurrently + with another function operating on the same device. The call is + idempotent and is safe to invoke when no read is in flight. + + Recommended usage to cleanly shut down a dedicated read thread: + @code + hid_read_interrupt(dev); + // join the read thread + hid_close(dev); + @endcode + + @ingroup API + @param dev A device handle returned from hid_open(). + + @returns + This function returns 0 on success and -1 on error. + Call hid_error(dev) to get the failure reason. + */ + int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev); + + /** @brief Query whether the read pipeline is currently interrupted. + + Since version 0.16.0, @ref HID_API_VERSION >= HID_API_MAKE_VERSION(0, 16, 0) + + Returns the current interrupt state set by @ref hid_read_interrupt + and @ref hid_read_clear_interrupt. Suitable for cross-thread + observation (read with acquire semantics). + + @ingroup API + @param dev A device handle returned from hid_open(). + + @returns + 1 if the read pipeline is interrupted, 0 if not, -1 on error. + Call hid_error(dev) to get the failure reason. + */ + int HID_API_EXPORT HID_API_CALL hid_is_read_interrupted(hid_device *dev); + + /** @brief Clear the read-interrupt state, allowing reads to resume. + + Since version 0.16.0, @ref HID_API_VERSION >= HID_API_MAKE_VERSION(0, 16, 0) + + After this call, subsequent hid_read()/hid_read_timeout() calls + operate normally. Any data that the device buffered during the + interrupted period may be returned by subsequent reads, subject + to a (backend-specific) buffer capacity. For a fresh-start + behavior, the caller may drain the buffered data with a loop of + hid_read_timeout(dev, ..., 0) calls before resuming the normal + read loop. + + Recommended use: call only when no hid_read*() call is in flight + on @p dev. The timing of an in-flight read returning -1 versus + a concurrent clear-interrupt taking effect is undefined. + + @ingroup API + @param dev A device handle returned from hid_open(). + + @returns + This function returns 0 on success and -1 on error. + Call hid_error(dev) to get the failure reason. + */ + int HID_API_EXPORT HID_API_CALL hid_read_clear_interrupt(hid_device *dev); + /** @brief Send a Feature report to the device. Feature reports are sent over the Control endpoint as a diff --git a/libusb/hid.c b/libusb/hid.c index 0b95d7e2..d895b624 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -125,6 +125,7 @@ struct hid_device_ { /* Read thread objects */ hidapi_thread_state thread_state; int shutdown_thread; + int read_interrupted; int transfer_loop_finished; struct libusb_transfer *transfer; @@ -1681,14 +1682,23 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t goto ret; } + if (dev->read_interrupted) { + bytes_read = -1; + register_read_error(dev, "hid_read(_timeout): operation interrupted"); + goto ret; + } + if (milliseconds == -1) { /* Blocking */ - while (!dev->input_reports && !dev->shutdown_thread) { + while (!dev->input_reports && !dev->shutdown_thread && !dev->read_interrupted) { hidapi_thread_cond_wait(&dev->thread_state); } if (dev->input_reports) { bytes_read = return_data(dev, data, length); } + else if (dev->read_interrupted) { + register_read_error(dev, "hid_read(_timeout): operation interrupted"); + } else { /* Woken up by shutdown_thread without data. */ register_read_error(dev, "hid_read(_timeout): read thread terminated"); @@ -1701,13 +1711,17 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t hidapi_thread_gettime(&ts); hidapi_thread_addtime(&ts, milliseconds); - while (!dev->input_reports && !dev->shutdown_thread) { + while (!dev->input_reports && !dev->shutdown_thread && !dev->read_interrupted) { res = hidapi_thread_cond_timedwait(&dev->thread_state, &ts); if (res == 0) { if (dev->input_reports) { bytes_read = return_data(dev, data, length); break; } + if (dev->read_interrupted) { + register_read_error(dev, "hid_read(_timeout): operation interrupted"); + break; + } if (dev->shutdown_thread) { register_read_error(dev, "hid_read(_timeout): read thread terminated"); break; @@ -1763,6 +1777,34 @@ int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) } +int HID_API_EXPORT hid_read_interrupt(hid_device *dev) +{ + hidapi_thread_mutex_lock(&dev->thread_state); + dev->read_interrupted = 1; + hidapi_thread_cond_broadcast(&dev->thread_state); + hidapi_thread_mutex_unlock(&dev->thread_state); + return 0; +} + + +int HID_API_EXPORT hid_is_read_interrupted(hid_device *dev) +{ + hidapi_thread_mutex_lock(&dev->thread_state); + int v = dev->read_interrupted; + hidapi_thread_mutex_unlock(&dev->thread_state); + return v; +} + + +int HID_API_EXPORT hid_read_clear_interrupt(hid_device *dev) +{ + hidapi_thread_mutex_lock(&dev->thread_state); + dev->read_interrupted = 0; + hidapi_thread_mutex_unlock(&dev->thread_state); + return 0; +} + + int HID_API_EXPORT hid_send_feature_report(hid_device *dev, const unsigned char *data, size_t length) { int res = -1; diff --git a/linux/hid.c b/linux/hid.c index a4dc26f4..e33c4b20 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -33,6 +33,8 @@ #include #include #include +#include +#include #include #include @@ -77,6 +79,8 @@ struct hid_device_ { wchar_t *last_error_str; wchar_t *last_read_error_str; struct hid_device_info* device_info; + int interrupt_efd; + volatile int interrupted; }; static struct hid_api_version api_version = { @@ -100,6 +104,12 @@ static hid_device *new_hid_device(void) dev->last_error_str = NULL; dev->last_read_error_str = NULL; dev->device_info = NULL; + dev->interrupted = 0; + dev->interrupt_efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); + if (dev->interrupt_efd < 0) { + free(dev); + return NULL; + } return dev; } @@ -1103,7 +1113,7 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path) } else { /* Unable to open a device. */ - free(dev); + hid_close(dev); register_global_error_format("Failed to open a device with path '%s': %s", path, strerror(errno)); return NULL; } @@ -1139,6 +1149,11 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t /* Set device error to none */ register_error_str(&dev->last_read_error_str, NULL); + if (dev->interrupted) { + register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); + return -1; + } + int bytes_read; if (milliseconds >= 0) { @@ -1149,12 +1164,15 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t properly report device disconnection through read() when in non-blocking mode. */ int ret; - struct pollfd fds; - - fds.fd = dev->device_handle; - fds.events = POLLIN; - fds.revents = 0; - ret = poll(&fds, 1, milliseconds); + struct pollfd fds[2]; + + fds[0].fd = dev->device_handle; + fds[0].events = POLLIN; + fds[0].revents = 0; + fds[1].fd = dev->interrupt_efd; + fds[1].events = POLLIN; + fds[1].revents = 0; + ret = poll(fds, 2, milliseconds); if (ret == 0) { /* Timeout */ return ret; @@ -1164,15 +1182,24 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t register_error_str(&dev->last_read_error_str, strerror(errno)); return ret; } - else { - /* Check for errors on the file descriptor. This will - indicate a device disconnection. */ - if (fds.revents & (POLLERR | POLLHUP | POLLNVAL)) { - // We cannot use strerror() here as no -1 was returned from poll(). - errno = EIO; - register_error_str(&dev->last_read_error_str, "hid_read_timeout: unexpected poll error (device disconnected)"); - return -1; - } + + /* Device disconnection takes priority over the interrupt. */ + if (fds[0].revents & (POLLERR | POLLHUP | POLLNVAL)) { + // We cannot use strerror() here as no -1 was returned from poll(). + errno = EIO; + register_error_str(&dev->last_read_error_str, "hid_read_timeout: unexpected poll error (device disconnected)"); + return -1; + } + + if (fds[1].revents & POLLIN) { + register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); + return -1; + } + + if (!(fds[0].revents & POLLIN)) { + /* Neither fd is readable yet still poll() returned > 0; + treat as no data available. */ + return 0; } } @@ -1209,6 +1236,29 @@ int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) return 0; /* Success */ } +int HID_API_EXPORT hid_read_interrupt(hid_device *dev) +{ + dev->interrupted = 1; + uint64_t one = 1; + ssize_t written = write(dev->interrupt_efd, &one, sizeof(one)); + (void)written; + return 0; +} + +int HID_API_EXPORT hid_is_read_interrupted(hid_device *dev) +{ + return dev->interrupted; +} + +int HID_API_EXPORT hid_read_clear_interrupt(hid_device *dev) +{ + dev->interrupted = 0; + uint64_t v; + ssize_t got = read(dev->interrupt_efd, &v, sizeof(v)); + (void)got; + return 0; +} + int HID_API_EXPORT hid_send_feature_report(hid_device *dev, const unsigned char *data, size_t length) { @@ -1292,6 +1342,8 @@ void HID_API_EXPORT hid_close(hid_device *dev) return; close(dev->device_handle); + if (dev->interrupt_efd >= 0) + close(dev->interrupt_efd); free(dev->last_error_str); free(dev->last_read_error_str); diff --git a/mac/hid.c b/mac/hid.c index a91bc190..4c2e840c 100644 --- a/mac/hid.c +++ b/mac/hid.c @@ -141,6 +141,7 @@ struct hid_device_ { pthread_barrier_t barrier; /* Ensures correct startup sequence */ pthread_barrier_t shutdown_barrier; /* Ensures correct shutdown sequence */ int shutdown_thread; + int read_interrupted; wchar_t *last_error_str; wchar_t *last_read_error_str; }; @@ -163,6 +164,7 @@ static hid_device *new_hid_device(void) dev->input_reports = NULL; dev->device_info = NULL; dev->shutdown_thread = 0; + dev->read_interrupted = 0; dev->last_error_str = NULL; dev->last_read_error_str = NULL; @@ -1220,7 +1222,7 @@ static int cond_wait(hid_device *dev, pthread_cond_t *cond, pthread_mutex_t *mut to sleep. See the pthread_cond_timedwait() man page for details. */ - if (dev->shutdown_thread || dev->disconnected) { + if (dev->shutdown_thread || dev->disconnected || dev->read_interrupted) { return -1; } } @@ -1241,7 +1243,7 @@ static int cond_timedwait(hid_device *dev, pthread_cond_t *cond, pthread_mutex_t to sleep. See the pthread_cond_timedwait() man page for details. */ - if (dev->shutdown_thread || dev->disconnected) { + if (dev->shutdown_thread || dev->disconnected || dev->read_interrupted) { return -1; } } @@ -1286,6 +1288,12 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t goto ret; } + if (dev->read_interrupted) { + bytes_read = -1; + register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); + goto ret; + } + /* There is no data. Go to sleep and wait for data. */ if (milliseconds == -1) { @@ -1294,7 +1302,10 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t res = cond_wait(dev, &dev->condition, &dev->mutex); if (res == 0) bytes_read = return_data(dev, data, length); - else { + else if (dev->read_interrupted) { + register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); + bytes_read = -1; + } else { /* There was an error, or a device disconnection. */ register_error_str(&dev->last_read_error_str, "hid_read_timeout: error waiting for more data"); bytes_read = -1; @@ -1319,6 +1330,9 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t bytes_read = return_data(dev, data, length); } else if (res == ETIMEDOUT) { bytes_read = 0; + } else if (dev->read_interrupted) { + register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); + bytes_read = -1; } else { register_error_str(&dev->last_read_error_str, "hid_read_timeout: error waiting for more data"); bytes_read = -1; @@ -1355,6 +1369,31 @@ int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) return 0; } +int HID_API_EXPORT hid_read_interrupt(hid_device *dev) +{ + pthread_mutex_lock(&dev->mutex); + dev->read_interrupted = 1; + pthread_cond_broadcast(&dev->condition); + pthread_mutex_unlock(&dev->mutex); + return 0; +} + +int HID_API_EXPORT hid_is_read_interrupted(hid_device *dev) +{ + pthread_mutex_lock(&dev->mutex); + int v = dev->read_interrupted; + pthread_mutex_unlock(&dev->mutex); + return v; +} + +int HID_API_EXPORT hid_read_clear_interrupt(hid_device *dev) +{ + pthread_mutex_lock(&dev->mutex); + dev->read_interrupted = 0; + pthread_mutex_unlock(&dev->mutex); + return 0; +} + int HID_API_EXPORT hid_send_feature_report(hid_device *dev, const unsigned char *data, size_t length) { return set_report(dev, kIOHIDReportTypeFeature, data, length); diff --git a/netbsd/hid.c b/netbsd/hid.c index a9fca67c..801f07eb 100644 --- a/netbsd/hid.c +++ b/netbsd/hid.c @@ -52,9 +52,15 @@ struct hid_device_ { wchar_t *last_read_error_str; struct hid_device_info *device_info; size_t poll_handles_length; - struct pollfd poll_handles[256]; + /* poll_handles[0 .. poll_handles_length-1] are the uhid device fds. + poll_handles[poll_handles_length] is reserved for interrupt_pipe[0], + so poll() can be called directly on this array with a count of + poll_handles_length + 1, with no per-call copy. */ + struct pollfd poll_handles[HIDAPI_MAX_CHILD_DEVICES + 1]; int report_handles[256]; char path[USB_MAX_DEVNAMELEN]; + int interrupt_pipe[2]; + volatile int interrupted; }; struct hid_enumerate_data { @@ -825,6 +831,17 @@ HID_API_EXPORT hid_device * HID_API_CALL hid_open_path(const char *path) goto err_0; } + dev->interrupt_pipe[0] = -1; + dev->interrupt_pipe[1] = -1; + if (pipe(dev->interrupt_pipe) == -1) { + register_global_error_format("failed to create interrupt pipe: %s", strerror(errno)); + goto err_1; + } + fcntl(dev->interrupt_pipe[0], F_SETFL, O_NONBLOCK); + fcntl(dev->interrupt_pipe[1], F_SETFL, O_NONBLOCK); + fcntl(dev->interrupt_pipe[0], F_SETFD, FD_CLOEXEC); + fcntl(dev->interrupt_pipe[1], F_SETFD, FD_CLOEXEC); + drvctl = open(DRVCTLDEV, O_RDONLY | O_CLOEXEC); if (drvctl == -1) { register_global_error_format("failed to open drvctl: %s", strerror(errno)); @@ -875,6 +892,10 @@ HID_API_EXPORT hid_device * HID_API_CALL hid_open_path(const char *path) dev->device_handle = uhid; } + dev->poll_handles[dev->poll_handles_length].fd = dev->interrupt_pipe[0]; + dev->poll_handles[dev->poll_handles_length].events = POLLIN; + dev->poll_handles[dev->poll_handles_length].revents = 0; + dev->blocking = 1; dev->last_error_str = NULL; dev->device_info = NULL; @@ -889,6 +910,10 @@ HID_API_EXPORT hid_device * HID_API_CALL hid_open_path(const char *path) err_2: close(drvctl); err_1: + if (dev->interrupt_pipe[0] >= 0) + close(dev->interrupt_pipe[0]); + if (dev->interrupt_pipe[1] >= 0) + close(dev->interrupt_pipe[1]); free(dev); err_0: return NULL; @@ -903,12 +928,16 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char { int res; size_t i; - struct pollfd *ph; ssize_t n; register_device_read_error(dev, NULL); - res = poll(dev->poll_handles, dev->poll_handles_length, milliseconds); + if (dev->interrupted) { + register_device_read_error(dev, "hid_read_timeout: operation interrupted"); + return -1; + } + + res = poll(dev->poll_handles, dev->poll_handles_length + 1, milliseconds); if (res == -1) { register_device_read_error_format(dev, "error while polling: %s", strerror(errno)); return -1; @@ -917,22 +946,25 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char if (res == 0) return 0; - for (i = 0; i < dev->poll_handles_length; i++) { - ph = &dev->poll_handles[i]; + if (dev->poll_handles[dev->poll_handles_length].revents & POLLIN) { + register_device_read_error(dev, "hid_read_timeout: operation interrupted"); + return -1; + } - if (ph->revents & (POLLERR | POLLHUP | POLLNVAL)) { + for (i = 0; i < dev->poll_handles_length; i++) { + if (dev->poll_handles[i].revents & (POLLERR | POLLHUP | POLLNVAL)) { register_device_read_error(dev, "device IO error while polling"); return -1; } - if (ph->revents & POLLIN) + if (dev->poll_handles[i].revents & POLLIN) break; } if (i == dev->poll_handles_length) return 0; - n = read(ph->fd, data, length); + n = read(dev->poll_handles[i].fd, data, length); if (n == -1) { if (errno == EAGAIN || errno == EINPROGRESS) n = 0; @@ -961,6 +993,35 @@ int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonbloc return 0; } +int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev) +{ + dev->interrupted = 1; + const char one = 1; + ssize_t written = write(dev->interrupt_pipe[1], &one, 1); + (void)written; + return 0; +} + +int HID_API_EXPORT HID_API_CALL hid_is_read_interrupted(hid_device *dev) +{ + return dev->interrupted; +} + +int HID_API_EXPORT HID_API_CALL hid_read_clear_interrupt(hid_device *dev) +{ + dev->interrupted = 0; + char buf[64]; + ssize_t got; + do { + /* There might be more than 64 calls to hid_read_interrupt, + before calling hid_read_clear_interrupt - + need to make sure to drain the pipe completely */ + got = read(dev->interrupt_pipe[0], buf, sizeof(buf)); + } while (got > 0); + (void)got; + return 0; +} + int HID_API_EXPORT HID_API_CALL hid_send_feature_report(hid_device *dev, const unsigned char *data, size_t length) { return set_report(dev, data, length, UHID_FEATURE_REPORT); @@ -994,6 +1055,11 @@ void HID_API_EXPORT HID_API_CALL hid_close(hid_device *dev) for (size_t i = 0; i < dev->poll_handles_length; i++) close(dev->poll_handles[i].fd); + if (dev->interrupt_pipe[0] >= 0) + close(dev->interrupt_pipe[0]); + if (dev->interrupt_pipe[1] >= 0) + close(dev->interrupt_pipe[1]); + free(dev); } diff --git a/windows/hid.c b/windows/hid.c index 1e27f10a..a11da24e 100644 --- a/windows/hid.c +++ b/windows/hid.c @@ -203,6 +203,8 @@ struct hid_device_ { char *read_buf; OVERLAPPED ol; OVERLAPPED write_ol; + HANDLE interrupt_event; + volatile LONG interrupted; struct hid_device_info* device_info; DWORD write_timeout_ms; }; @@ -230,6 +232,8 @@ static hid_device *new_hid_device() dev->ol.hEvent = CreateEvent(NULL, FALSE, FALSE /*initial state f=nonsignaled*/, NULL); memset(&dev->write_ol, 0, sizeof(dev->write_ol)); dev->write_ol.hEvent = CreateEvent(NULL, FALSE, FALSE /*initial state f=nonsignaled*/, NULL); + dev->interrupt_event = CreateEvent(NULL, TRUE /*manual reset*/, FALSE /*initial state nonsignaled*/, NULL); + dev->interrupted = 0; dev->device_info = NULL; dev->write_timeout_ms = 1000; @@ -240,6 +244,7 @@ static void free_hid_device(hid_device *dev) { CloseHandle(dev->ol.hEvent); CloseHandle(dev->write_ol.hEvent); + CloseHandle(dev->interrupt_event); CloseHandle(dev->device_handle); free(dev->last_error_str); free(dev->last_read_error_str); @@ -1173,6 +1178,11 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char register_string_error_to_buffer(&dev->last_read_error_str, NULL); + if (InterlockedCompareExchange(&dev->interrupted, 0, 0)) { + register_string_error_to_buffer(&dev->last_read_error_str, L"hid_read_timeout: operation interrupted"); + return -1; + } + /* Copy the handle for convenience. */ HANDLE ev = dev->ol.hEvent; @@ -1200,9 +1210,16 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char } if (overlapped) { - /* See if there is any data yet. */ - res = WaitForSingleObject(ev, milliseconds >= 0 ? (DWORD)milliseconds : INFINITE); - if (res != WAIT_OBJECT_0) { + HANDLE waits[2] = { ev, dev->interrupt_event }; + DWORD wr = WaitForMultipleObjects(2, waits, FALSE, milliseconds >= 0 ? (DWORD)milliseconds : INFINITE); + if (wr == WAIT_OBJECT_0 + 1) { + /* Interrupt fired and no data is ready. The pending ReadFile is + left in flight; it will resume on the next hid_read_timeout() + call after hid_read_clear_interrupt(). */ + register_string_error_to_buffer(&dev->last_read_error_str, L"hid_read_timeout: operation interrupted"); + return -1; + } + if (wr != WAIT_OBJECT_0) { /* There was no data this time. Return zero bytes available, but leave the Overlapped I/O running. */ return 0; @@ -1257,6 +1274,28 @@ HID_API_EXPORT const wchar_t * HID_API_CALL hid_read_error(hid_device *dev) return dev->last_read_error_str; } +int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev) +{ + InterlockedExchange(&dev->interrupted, 1); + SetEvent(dev->interrupt_event); + return 0; +} + +int HID_API_EXPORT HID_API_CALL hid_is_read_interrupted(hid_device *dev) +{ + // TODO: very common but not the most efficient way to check this, + // move to C11 atomics when the time comes. + // For now this is the most portable and efficient enought. + return InterlockedCompareExchange(&dev->interrupted, 0, 0) ? 1 : 0; +} + +int HID_API_EXPORT HID_API_CALL hid_read_clear_interrupt(hid_device *dev) +{ + InterlockedExchange(&dev->interrupted, 0); + ResetEvent(dev->interrupt_event); + return 0; +} + int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonblock) { dev->blocking = !nonblock; From 17fcf9a1e066814b204dee1a9ba6124de302142b Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Tue, 5 May 2026 19:15:13 +0300 Subject: [PATCH 2/2] Address PR #799 review comments (#800) --- hid_read_test/CMakeLists.txt | 20 ++++++++++++++++---- hid_read_test/main.cpp | 23 ++++++++++++++++++----- linux/hid.c | 18 ++++++++++-------- netbsd/hid.c | 10 +++++----- windows/hid.c | 4 ++-- 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/hid_read_test/CMakeLists.txt b/hid_read_test/CMakeLists.txt index 32e300f4..77f97669 100644 --- a/hid_read_test/CMakeLists.txt +++ b/hid_read_test/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR) +cmake_minimum_required(VERSION 3.1.3...3.25 FATAL_ERROR) project(hid_read_test CXX) if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) @@ -19,20 +19,32 @@ set(HIDAPI_HID_READ_TEST_TARGETS) if(NOT WIN32 AND NOT APPLE AND CMAKE_SYSTEM_NAME MATCHES "Linux") if(TARGET hidapi::hidraw) add_executable(hid_read_test_hidraw main.cpp) - target_compile_features(hid_read_test_hidraw PRIVATE cxx_std_11) + set_target_properties(hid_read_test_hidraw PROPERTIES + CXX_STANDARD 11 + CXX_STANDARD_REQUIRED ON + CXX_EXTENSIONS OFF + ) target_link_libraries(hid_read_test_hidraw PRIVATE hidapi::hidraw Threads::Threads) list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test_hidraw) endif() if(TARGET hidapi::libusb) add_executable(hid_read_test_libusb main.cpp) - target_compile_features(hid_read_test_libusb PRIVATE cxx_std_11) + set_target_properties(hid_read_test_libusb PROPERTIES + CXX_STANDARD 11 + CXX_STANDARD_REQUIRED ON + CXX_EXTENSIONS OFF + ) target_compile_definitions(hid_read_test_libusb PRIVATE USING_HIDAPI_LIBUSB) target_link_libraries(hid_read_test_libusb PRIVATE hidapi::libusb Threads::Threads) list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test_libusb) endif() else() add_executable(hid_read_test main.cpp) - target_compile_features(hid_read_test PRIVATE cxx_std_11) + set_target_properties(hid_read_test PROPERTIES + CXX_STANDARD 11 + CXX_STANDARD_REQUIRED ON + CXX_EXTENSIONS OFF + ) target_link_libraries(hid_read_test PRIVATE hidapi::hidapi Threads::Threads) list(APPEND HIDAPI_HID_READ_TEST_TARGETS hid_read_test) endif() diff --git a/hid_read_test/main.cpp b/hid_read_test/main.cpp index 454b5a3b..06db214f 100644 --- a/hid_read_test/main.cpp +++ b/hid_read_test/main.cpp @@ -24,9 +24,13 @@ #include #include +#ifndef _WIN32 +#include +#endif + namespace { -std::atomic g_dev{nullptr}; +std::atomic g_terminate{false}; std::string timestamp_now() { @@ -50,8 +54,9 @@ std::string timestamp_now() extern "C" void on_signal(int) { - if (hid_device *d = g_dev.load(std::memory_order_acquire)) - hid_read_interrupt(d); + /* async-signal-safe: atomic store only. Main thread will call + hid_read_interrupt() once cin.get() returns from EINTR. */ + g_terminate.store(true, std::memory_order_release); } void read_thread_fn(hid_device *dev) @@ -122,12 +127,20 @@ int main(int argc, char **argv) hid_exit(); return 1; } - g_dev.store(dev, std::memory_order_release); - +#ifdef _WIN32 std::signal(SIGINT, on_signal); #ifdef SIGTERM std::signal(SIGTERM, on_signal); #endif +#else + /* Use sigaction without SA_RESTART so cin.get() returns on signal. */ + struct sigaction sa; + sa.sa_handler = on_signal; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + sigaction(SIGINT, &sa, nullptr); + sigaction(SIGTERM, &sa, nullptr); +#endif std::cout << "Reading from VID=" << std::hex << std::setw(4) << std::setfill('0') << vid << " PID=" << std::setw(4) << pid << std::dec diff --git a/linux/hid.c b/linux/hid.c index e33c4b20..659f268c 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -80,7 +80,7 @@ struct hid_device_ { wchar_t *last_read_error_str; struct hid_device_info* device_info; int interrupt_efd; - volatile int interrupted; + int interrupted; }; static struct hid_api_version api_version = { @@ -107,7 +107,9 @@ static hid_device *new_hid_device(void) dev->interrupted = 0; dev->interrupt_efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); if (dev->interrupt_efd < 0) { + int saved_errno = errno; free(dev); + errno = saved_errno; return NULL; } @@ -1091,8 +1093,7 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path) dev = new_hid_device(); if (!dev) { - errno = ENOMEM; - register_global_error("Couldn't allocate memory"); + register_global_error_format("Failed to create hid_device: %s", strerror(errno)); return NULL; } @@ -1149,7 +1150,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t /* Set device error to none */ register_error_str(&dev->last_read_error_str, NULL); - if (dev->interrupted) { + if (__atomic_load_n(&dev->interrupted, __ATOMIC_ACQUIRE)) { register_error_str(&dev->last_read_error_str, "hid_read_timeout: operation interrupted"); return -1; } @@ -1238,7 +1239,7 @@ int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) int HID_API_EXPORT hid_read_interrupt(hid_device *dev) { - dev->interrupted = 1; + __atomic_store_n(&dev->interrupted, 1, __ATOMIC_RELEASE); uint64_t one = 1; ssize_t written = write(dev->interrupt_efd, &one, sizeof(one)); (void)written; @@ -1247,12 +1248,12 @@ int HID_API_EXPORT hid_read_interrupt(hid_device *dev) int HID_API_EXPORT hid_is_read_interrupted(hid_device *dev) { - return dev->interrupted; + return __atomic_load_n(&dev->interrupted, __ATOMIC_ACQUIRE); } int HID_API_EXPORT hid_read_clear_interrupt(hid_device *dev) { - dev->interrupted = 0; + __atomic_store_n(&dev->interrupted, 0, __ATOMIC_RELEASE); uint64_t v; ssize_t got = read(dev->interrupt_efd, &v, sizeof(v)); (void)got; @@ -1341,7 +1342,8 @@ void HID_API_EXPORT hid_close(hid_device *dev) if (!dev) return; - close(dev->device_handle); + if (dev->device_handle >= 0) + close(dev->device_handle); if (dev->interrupt_efd >= 0) close(dev->interrupt_efd); diff --git a/netbsd/hid.c b/netbsd/hid.c index 801f07eb..383fcb11 100644 --- a/netbsd/hid.c +++ b/netbsd/hid.c @@ -60,7 +60,7 @@ struct hid_device_ { int report_handles[256]; char path[USB_MAX_DEVNAMELEN]; int interrupt_pipe[2]; - volatile int interrupted; + int interrupted; }; struct hid_enumerate_data { @@ -932,7 +932,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char register_device_read_error(dev, NULL); - if (dev->interrupted) { + if (__atomic_load_n(&dev->interrupted, __ATOMIC_ACQUIRE)) { register_device_read_error(dev, "hid_read_timeout: operation interrupted"); return -1; } @@ -995,7 +995,7 @@ int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonbloc int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev) { - dev->interrupted = 1; + __atomic_store_n(&dev->interrupted, 1, __ATOMIC_RELEASE); const char one = 1; ssize_t written = write(dev->interrupt_pipe[1], &one, 1); (void)written; @@ -1004,12 +1004,12 @@ int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev) int HID_API_EXPORT HID_API_CALL hid_is_read_interrupted(hid_device *dev) { - return dev->interrupted; + return __atomic_load_n(&dev->interrupted, __ATOMIC_ACQUIRE); } int HID_API_EXPORT HID_API_CALL hid_read_clear_interrupt(hid_device *dev) { - dev->interrupted = 0; + __atomic_store_n(&dev->interrupted, 0, __ATOMIC_RELEASE); char buf[64]; ssize_t got; do { diff --git a/windows/hid.c b/windows/hid.c index a11da24e..bbf196dd 100644 --- a/windows/hid.c +++ b/windows/hid.c @@ -1283,9 +1283,9 @@ int HID_API_EXPORT HID_API_CALL hid_read_interrupt(hid_device *dev) int HID_API_EXPORT HID_API_CALL hid_is_read_interrupted(hid_device *dev) { - // TODO: very common but not the most efficient way to check this, + // TODO: common but not the most efficient way to check this; // move to C11 atomics when the time comes. - // For now this is the most portable and efficient enought. + // For now this is portable and efficient enough. return InterlockedCompareExchange(&dev->interrupted, 0, 0) ? 1 : 0; }