RFC: Add hid_read_interrupt API for thread-safe read cancellation by Youw · Pull Request #799 · libusb/hidapi

Youw · 2026-04-26T22:28:54Z

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

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

Copilot

Pull request overview

This PR adds a new public HIDAPI mechanism to asynchronously interrupt a blocked hid_read()/hid_read_timeout() call in a thread-safe way (intended to allow clean shutdown of dedicated reader threads without short timeouts/busy polling), and introduces a small hid_read_test utility to exercise the behavior.

Changes:

Adds hid_read_interrupt(), hid_is_read_interrupted(), and hid_read_clear_interrupt() to the public header and implements them across all backends.
Updates backend hid_read_timeout() implementations to return -1 with an “operation interrupted” read error when interruption is requested.
Adds a new optional hid_read_test tool and a top-level CMake option to build it.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`windows/hid.c`	Implements interrupt via a manual-reset event + interlocked flag and integrates it into `WaitForMultipleObjects()` in `hid_read_timeout()`.
`netbsd/hid.c`	Implements interrupt via a nonblocking pipe added to the `poll()` fd set, plus a sticky interrupt flag.
`mac/hid.c`	Implements interrupt via a mutex-protected flag + condition broadcast integrated into the read wait loops.
`linux/hid.c`	Implements interrupt via `eventfd` added to `poll()` plus a sticky interrupt flag; adjusts failure cleanup path to call `hid_close()`.
`libusb/hid.c`	Implements interrupt via a mutex-protected flag + condition broadcast integrated into the read wait loops.
`hidapi/hidapi.h`	Adds the public API declarations and documents the intended thread-safety/semantics.
`hid_read_test/main.cpp`	New command-line tool that opens a device, runs a blocking read thread, and interrupts on Enter/Ctrl+C.
`hid_read_test/CMakeLists.txt`	Adds build/install rules for the new test tool (standalone or as a subdir).
`CMakeLists.txt`	Adds `HIDAPI_BUILD_HID_READ_TEST` option to include the new tool in the build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-26T22:34:46Z

+	// 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.


Spelling/grammar in the comment: “enought” → “enough”, and “very common” likely intended as “common”.

Suggested change

// 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.

// TODO: common but not the most efficient way to check this;

// move to C11 atomics when the time comes.

// For now this is portable and efficient enough.

Copilot · 2026-04-26T22:34:46Z

 	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;


In the open-failure path, calling hid_close(dev) before formatting the error will clobber errno because hid_close() unconditionally calls close(dev->device_handle) even when device_handle == -1. That makes the reported error message likely show EBADF instead of the real open() failure. Capture/restore errno around hid_close(), or make hid_close() skip close() when the handle is negative.

Copilot · 2026-04-26T22:34:47Z

+	if (dev->interrupt_efd < 0) {
+		free(dev);


new_hid_device() can now fail because eventfd() failed (e.g., EMFILE/ENFILE/EPERM), but the caller (hid_open_path) still treats any NULL return as an allocation failure and overwrites errno/global error accordingly. Consider preserving the real errno from eventfd() and reporting a more accurate error (or providing a fallback wakeup mechanism) so device opens don’t fail with a misleading "Couldn't allocate memory" message.

Suggested change

if (dev->interrupt_efd < 0) {

free(dev);

if (dev->interrupt_efd < 0) {

int saved_errno = errno;

free(dev);

errno = saved_errno;

Copilot · 2026-04-26T22:34:47Z

 	struct hid_device_info* device_info;
+	int interrupt_efd;
+	volatile int interrupted;
 };


dev->interrupted is accessed concurrently from hid_read_interrupt(), hid_read_clear_interrupt(), and hid_read_timeout() using plain (volatile) loads/stores. volatile does not make this thread-safe in C; this introduces a data race/UB and breaks the API guarantee that hid_read_interrupt() is safe to call concurrently with a blocked read. Use an actual atomic (C11 stdatomic.h, compiler atomics) or protect the flag with a mutex, so both the state and the wakeup mechanism have well-defined cross-thread semantics.

Copilot · 2026-04-26T22:34:47Z

 	char path[USB_MAX_DEVNAMELEN];
+	int interrupt_pipe[2];
+	volatile int interrupted;


dev->interrupted is read in hid_read_timeout() and written in hid_read_interrupt()/hid_read_clear_interrupt() without any synchronization. Marking it volatile does not make these operations atomic or race-free; this is undefined behavior if hid_read_interrupt() is called from another thread, which is the main purpose of the new API. Switch to an atomic flag or guard access with a mutex (and keep the pipe strictly for wakeups).

Copilot · 2026-04-26T22:34:47Z

+			and @ref hid_read_clear_interrupt. Suitable for cross-thread
+			observation (read with acquire semantics).


The comment promises cross-thread observation with acquire semantics, but at least the Linux and NetBSD backends implement the interrupt state as a plain int/volatile int without atomics or locking. Either strengthen the implementations to actually provide atomic/acquire semantics everywhere, or relax/reword this guarantee in the public header to match what is implemented.

Suggested change

and @ref hid_read_clear_interrupt. Suitable for cross-thread

observation (read with acquire semantics).

and @ref hid_read_clear_interrupt. This reports the library's

current interrupt flag for @p dev, but does not by itself provide

any additional cross-thread synchronization or memory-ordering

guarantees.

Copilot · 2026-04-26T22:34:48Z

+extern "C" void on_signal(int)
+{
+    if (hid_device *d = g_dev.load(std::memory_order_acquire))
+        hid_read_interrupt(d);
+}


on_signal() calls hid_read_interrupt() directly from a signal handler. That is not async-signal-safe on POSIX (e.g. mac/libusb implementations take a pthread mutex/condvar), and can deadlock or crash if the signal interrupts code holding the same lock. Consider making the handler only set an atomic flag (or write to a self-pipe) and have the main thread (or a dedicated signal-wait thread using sigwait) call hid_read_interrupt() from a safe context.

Copilot · 2026-04-26T22:34:48Z

@@ -0,0 +1,42 @@
+cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR)


The CMake minimum version here (3.8) is higher than the repository’s top-level minimum (3.1.3). Enabling HIDAPI_BUILD_HID_READ_TEST will therefore make the overall build require a newer CMake than advertised. Consider aligning this cmake_minimum_required() with the top-level requirement unless this tool genuinely needs newer CMake features.

Suggested change

cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR)

cmake_minimum_required(VERSION 3.1.3 FATAL_ERROR)

Youw requested a review from Copilot April 26, 2026 22:28

Youw changed the title ~~Add hid_read_interrupt API for thread-safe read cancellation~~ RFC: Add hid_read_interrupt API for thread-safe read cancellation Apr 26, 2026

Copilot started reviewing on behalf of Youw April 26, 2026 22:29 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

mcuee added the enhancement New feature or request label Apr 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Add hid_read_interrupt API for thread-safe read cancellation#799

RFC: Add hid_read_interrupt API for thread-safe read cancellation#799
Youw wants to merge 1 commit intomasterfrom
read-interrupt

Youw commented Apr 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Copilot AI Apr 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		and @ref hid_read_clear_interrupt. Suitable for cross-thread
		observation (read with acquire semantics).

-			and @ref hid_read_clear_interrupt. Suitable for cross-thread
-			observation (read with acquire semantics).
+			and @ref hid_read_clear_interrupt. This reports the library's
+			current interrupt flag for @p dev, but does not by itself provide
+			any additional cross-thread synchronization or memory-ordering
+			guarantees.

		@@ -0,0 +1,42 @@
		cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR)

	cmake_minimum_required(VERSION 3.8...3.25 FATAL_ERROR)
	cmake_minimum_required(VERSION 3.1.3 FATAL_ERROR)

Conversation

Youw commented Apr 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants