WIP: RFC: Hotplug implementation #674
No reviewers
Labels
No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-libusb/hidapi!674
Loading…
Reference in a new issue
No description provided.
Delete branch "connection-callback"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Resolves: #238
keeping it as draft for now
still need to fill up a description, etc.
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||would it be possible/feasible to postpone the processing of the event until libusb has closed the device? I know that might get complicate but otherwise enumerate would not work (at least as long as https://github.com/libusb/libusb/pull/1532 has not been merged).
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||I don't follow your thoughts here. Can you elaborate more details?
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||well, libusb opens the device and fires the callback, then hidapi tries to get infos from the device but may fail as it is still open by libusb.
if I run my example program with the libusb backend, I don't get e.g. manufacturer infos but with the hidraw backend I get them.
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||I was wrong, libusb does not open the device. but for some reason,
hid_enumerate_from_libusb()is not able to open the device in my case (LIBUSB_ERROR_ACCESS) when called by the hotplug handler. If I insert a small delay beforelibusb_open()it works.@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||Interesting finding...
As per documentation - it should be able to use libusb_open from a hotplug callback function.
What version of libusb are you using?
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||I use libusb v1.0.27 and this branch for hidapi. Everything can be found here: https://github.com/bearsh/hid/tree/feature/connection-callback including the test program. The project is a go-wrapper around hiadpi. The test application is in
cmd/hid-hotplug.maybe I should file a bug at libusb...I've created https://github.com/libusb/libusb/issues/1586@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||something like the following would solve my issue and is also suggested as workaround by @mcuee in https://github.com/libusb/libusb/issues/1586#issuecomment-2500248425
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||I do not know anything about Go, but let me try out the Go-wrapper over the weekend to see if I can reproduce the issue under Linux or not.
is there something I can help with to get this merged?
@Youw any chance the branch could be updated to the fresh master?
Also, is there any plan on actually finishing the feature & making it available in mainline? What's missing at this point and how can I help?
Sure, I'll update the branch.
Answering to last two comments: lets make a deal here: I'll start looking into this in more details very shortly (within days, at most weeks) - starting with the overall API design, etc. I'll post all my findings (and issues if any), and if anything needed to be adjusted/fixed - I'll probably be asking for help.
Also, I really hope we would have a good semi-automatic test for this as well (semi-automatic, since it would probably involve some physical device plug/unplug).
If all good when - will merge it into master.
This doesn't really describe my side of the deal, but - sure, sounds good. I'll do what I can to help in getting this ready.
This PR does already feature an update to the hidtest utility. The update makes it interactive & allows to test the callbacks. I don't know how to make it any more automatic.
(No I haven't looke at what the modified hidtest currently does, if it already does something like that - that might be good-enought)
I've updated this branch to latest master and fixed conflicts (hopefully right).
Next step - I'll review the API and test the usability.
I don't think my test app fits the description, and if the current hidtest is used in any automated testing scenario, I shall revert it to how it is in the current release. I'm not sure if the new interactive solution has any use, but it can be preserved under a different subfolder (will require a new cmake target, though).
As for the automated hotplug test, I'm not sure this is even necessary, but I can implement that too. I believe this can wait until after the review of the API - if the API is to change, the test will have to change too.
Hi, any news since?
We really would like to use this functionality in OpenRGB's upcoming 1.0 release. Is there any plan to merge this into a hidapi release in the near future? If not, we plan on creating a fork called hidapi-hotplug (just hidapi master with the changes from this branch) and building hidapi-hotplug packages to ship alongside OpenRGB 1.0.
I do plan to make OpenRGB 1.0 buildable against standard non-hotplug hidapi but without hotplug capability. Hotplug capability would be a major improvement in user experience for us and @k1-801 has been pushing for it for years. I've been hesitant because of lack of upstream support but I really want to find a way to get this in and maintaining a (hopefully temporary) fork seems like the best action right now.
I failed to find capacity to review this once again, and I apologize for that.
As of right now at least I've updated the branch to latest master.
I do not want to make any promices as to when I get my hands to it (so I don't break that promice again), but this is on my task list, I'm not abandoning the idea to get this done.
@CalcProgrammer1 does this mean you have tested the hotplug implementation in-depth? Did you test it with the libusb backend?
I still think something like https://github.com/libusb/hidapi/pull/674#discussion_r1860010559 is needed to make it fully functional...
I personally haven't, I tested the hidraw implementation.
However, @k1-801's been working on this hotplug system for years and has
kept it continuously rebased, and his implementation had support for
libusb-based devices as well has hidraw (which I haven't pulled into mine
yet but intend to). There are a handful of devices with odd or broken HID
descriptors that Linux's hidraw driver fails to properly detect, but we can
still access via the libusb-based hidapi.
On Thu, Jan 15, 2026 at 8:56 AM bearsh @.***> wrote:
I've updated my next_hotplug branch again on OpenRGB, see:
https://gitlab.com/CalcProgrammer1/OpenRGB/-/merge_requests/3162
I pulled in support for libusb-wrapped devices and can confirm that the libusb backend also seems to work. For some reason, certain devices (like my HyperX Quadcast S microphone) use HID but have a descriptor that causes hidraw not to detect it, so the only way to access on Linux is using the libusb backend. We have a wrapper for these devices in OpenRGB, so that it normally links against the hidraw backend and uses that for most devices, but we dynamically load the libusb backend and use a wrapper to call its version of the functions for these select few problem devices.
With this latest push to next_hotplug, I can hotplug (add device on plug in, remove device on unplug) my HyperX Quadcast S microphone via the libusb wrapper as well as a variety of Razer devices I've been using to test the hidraw path. All is working well on Linux. Have not tested other OSes yet.
I built Windows and MacOS builds of my fork today and got successful builds of OpenRGB (next_hotplug branch) on Windows and MacOS. I've confirmed both are working, both inserting and removing.
Pull request overview
This PR introduces cross-platform hotplug (device arrival/removal) support to hidapi, adding a public callback registration API and implementing platform-specific event backends (Windows cfgmgr32 notifications, macOS IOHIDManager, Linux udev monitor, libusb hotplug), plus extending
hidtestto exercise the new functionality.Changes:
hid_hotplug_register_callback/hid_hotplug_deregister_callback) and related types inhidapi.h.hidtestwith an interactive hotplug testing mode and stress scenarios.Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.
Show a summary per file
hidapi/hidapi.hwindows/hidapi_cfgmgr32.hwindows/hid.cmac/hid.clinux/hid.clibusb/hid.cnetbsd/hid.chidtest/test.c💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@ -261,6 +261,324 @@ extern "C" {*/The documentation says it is safe to call
hid_hotplug_deregister_callback()on an already deregistered callback. However, current backend implementations return -1 when the callback list is empty / not initialized, which contradicts that contract. Either adjust the implementations to treat “not found / already removed” as success (return 0), or update this documentation to match the actual behavior.@ -336,13 +348,238 @@ int main(int argc, char* argv[])}This printf string ends with
\n]n, which looks like a typo and will print a stray]instead of a newline sequence. It should likely be\n\n(or similar) to match the surrounding output formatting.@ -915,0 +1086,4 @@if ((vendor_id != 0x0 && vendor_id != dev_vid) ||(product_id != 0x0 && product_id != dev_pid)) {return NULL;}On LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT handling, the code frees removed
hid_device_infonodes withfree(info).hid_device_infocontains heap-allocated fields (path and strings), so this leaks memory. Sinceinfo->nextis set to NULL, usehid_free_enumeration(info)to free the node and its fields correctly.@ -1053,0 +1551,4 @@int result = -1;/* Remove this notification */If
calloc()for the queue message fails, the callback returns without releasing the earlierlibusb_ref_device(device)reference, leaking a libusb device refcount. Ensure the failure path callslibusb_unref_device(device)before returning.@ -1033,6 +1169,276 @@ void HID_API_EXPORT hid_free_enumeration(struct hid_device_info *devs)}On device removal handling in the udev hotplug thread, the code frees a removed
hid_device_infonode withfree(info). This leaks the heap-allocated members ofhid_device_info(path and strings) which are normally freed byhid_free_enumeration(). Sinceinfo->nextis already set to NULL, usehid_free_enumeration(info)to free the node correctly.@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);hid_internal_hotplug_cleanup()callspthread_join(hid_hotplug_context.thread, NULL)while it is intended to be invoked while holdinghid_hotplug_context.mutex. This cleanup can be reached from IOHID callbacks that are executed on the hotplug thread itself; in that case, joininghid_hotplug_context.threadfrom within the same thread will deadlock. Guard against self-join (e.g.,pthread_equal(pthread_self(), hid_hotplug_context.thread)) and/or redesign shutdown so the join happens from a different thread or the hotplug thread is detached.@ -564,1 +716,3 @@if (cur_dev == NULL) {/* A small trick to store an io_service_t tag along with hid_device_info for matching info with unplugged device */cur_dev_ex = (struct hid_device_info_ex *)calloc(1, sizeof(struct hid_device_info_ex));if (cur_dev_ex == NULL) {calloc()result (cur_dev_ex) is dereferenced before being NULL-checked (cur_dev = &(cur_dev_ex->info);). If allocation fails, this is an immediate NULL dereference. Checkcur_dev_exfor NULL before accessing its fields (and return NULL on failure).hid_exit()only callshid_internal_hotplug_exit()whenhid_mgris non-NULL. Since hotplug registration does not requirehid_mgrto be initialized (and callers may rely onhid_exit()to clean up regardless), this can leak hotplug threads/mutex state. Callhid_internal_hotplug_exit()unconditionally (or at least whenever the hotplug context is initialized).@ -813,1 +969,4 @@static void hid_internal_invoke_callbacks(struct hid_device_info *info, hid_hotplug_event event){pthread_mutex_lock(&hid_hotplug_context.mutex);On device removal, the code detaches a
hid_device_infonode fromhid_hotplug_context.devsand then frees it withfree(info). Eachhid_device_infocontains heap-allocated fields (path, strings), so this leaks memory; also this list was built bycreate_device_info*()which expectshid_free_enumeration()for cleanup. Setinfo->next = NULL(already done) and callhid_free_enumeration(info)instead.match_ref_to_info()is a file-local helper but is not declaredstatic, which unnecessarily exports a global symbol from this compilation unit (and increases the risk of symbol collisions). Make itstaticto keep it private to this file.When processing a device removal hotplug event, the code frees only the
struct hid_device_infonode withfree(device), buthid_internal_get_device_info()allocatespath,serial_number,manufacturer_string, andproduct_string. This leaks those allocations; use the existinghid_free_enumeration(device)(after ensuringdevice->nextis NULL) to free the node and its fields correctly.In the first-callback registration path,
hid_hotplug_context.devs = hid_enumerate(0, 0)can return NULL (e.g., ifhid_init()/ DLL resolution fails), but the function continues and callsCM_Register_Notification(...)anyway. That can crash in the failure case and also registers a callback even though initial enumeration failed. Consider callinghid_init()(and failing fast) at the start ofhid_hotplug_register_callback(), and/or checking the result ofhid_enumerate()and rolling back state on failure.On the error returns in the first-callback path (e.g.,
notify_handle != NULLorCM_Register_Notification(...) != CR_SUCCESS), the newly allocatedhotplug_cb(and thedevslist fromhid_enumerate) are not freed, andhid_hotplug_context.hotplug_cbsmay already be set tohotplug_cb. This leaves the hotplug context in a partially-initialized state and leaks memory; the error path should undo any state changes and free allocations before returning.@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);This one can be disregarded - not the intended use of the cleanup function.
@ -261,6 +261,324 @@ extern "C" {*/"Safe" as in "won't crash, just returns an error", not as in "will succeed"
Some more documentation based on current implementation: https://github.com/libusb/hidapi/pull/784
Are we happy with this in general?
I think this is a good one to be merged to git master.
I actually don't see any reason why it'd be not safe to call
hid_open_pathfrom a callback. Hotplug subsystem doesn't really intersect with what happens in device functions. Could you explain further, why it wouldn't be safe? The worst possible case is @bearsh 's scenario, but they already suggested a patch that works for them.The only thing to really worry about processing directly in the callback is the callback potentially taking too long to process, but that's not related to the device opening call per se.
Additionally, theFound a mention of that.hid_device_infopointer may not be safe to preserve as-is outside the callback (and so are string pointers inside it), and THAT I'd put in the docs. Any info of interest in it should be copied, as the structure may be deleted unexpectedly after the callback ends.#784 documentation and
hidtest.ccan be improved along the way for hotplug. This PR can go in first.@Youw
I think you can mark this PR as "Ready for review".
Minor warning when building under VS2026. Not related to this PR though (same for git master).
Maybe it is related to by build command (adapted from github action script). Not so sure here.
would it be possible to include the patch from https://github.com/libusb/hidapi/pull/674#discussion_r1860010559? or should I submit a separate PR once this is merged?
I agree that this might be a worthy addition, if during testing it revealed such a problem.
@ -915,0 +1083,4 @@unsigned short dev_vid = desc.idVendor;unsigned short dev_pid = desc.idProduct;if ((vendor_id != 0x0 && vendor_id != dev_vid) ||A general rule: a retry w/o knowing exactly a reason for the behavior is not a good thing - that is a high indication somewhere is a bug, or something fundamentally done wrong...
In any case - lets have it as a separate issue/PR, this PR has too many comments/threads, things easily get lost...
@k1-801 lets continue the conversation her: https://github.com/libusb/hidapi/pull/784/
@bearsh see: https://github.com/libusb/hidapi/pull/674#discussion_r3015074686
Before marking this a Ready for Review/Merging to master - at least https://github.com/libusb/hidapi/pull/783 needs to be reviewed/merged
@ -261,6 +261,324 @@ extern "C" {*/while we want this to be true, I have a few conserns:
why do we wan't to make this possible? one could simply return non-zero from a callback function, and that would effectively deregister, w/o a need to call
deregistermanually...register/deregisterfunctions are not really thread-safe with regard to each other, mostly because context/mutex initisalisation in not thread-safe. even if we allow callingderegisterfrom within the callback, there should be a big NOTE regarding the fact, that all calls toregister/deregister, including the one from withing the callback function - should be serialized / protected by a mutex or so.@ -261,6 +261,324 @@ extern "C" {*/It could be deregistering a different callback than the one currently running, OR the user could need to perform certain actions after the callback is deregistered.
@ -261,6 +261,324 @@ extern "C" {*/True. We do need such a note.
Some thread safety did actually exist there and was removed later during review.
@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);@copilot elaborate in which scenario ths is possible
@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);Here's the exact call chain that causes the deadlock:
The IOHIDManager is scheduled with
CFRunLoopGetCurrent()insidehotplug_thread()(line 1113), so all IOHID removal callbacks run on the hotplug thread.Scenario — a user callback self-deregisters on device removal:
The key:
hid_internal_invoke_callbacks()resetsmutex_in_useto 0 before returning (line 997), so themutex_in_useguard inhid_internal_hotplug_cleanup()(line 553) does not prevent the join when the path comes from an IOHID callback.The fix is to guard against self-join using
pthread_equal(pthread_self(), hid_hotplug_context.thread)as suggested — if we're already on the hotplug thread, usepthread_detachinstead so resources are reclaimed on exit.https://github.com/libusb/hidapi/issues/785
@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);this is not to be disregarded
I'd even say we're back to a question - do we really want to allow deregister from a callback thread - things like this are dangerous and wer're luch we found it during the review and not later
moved conversation here: https://github.com/libusb/hidapi/issues/786
@ -261,6 +261,324 @@ extern "C" {*/There is also things like: https://github.com/libusb/hidapi/pull/674#discussion_r3015415035 that needs to be resolved first, and taken into account in general
@ -467,0 +574,4 @@if (!hid_hotplug_context.mutex_ready) {/* Initialize the mutex as recursive */pthread_mutexattr_t attr;pthread_mutexattr_init(&attr);I'm okay with removing the comment, even if such a scenario wouldn't cause issues on most platforms. Support for this specific use case was added explicitly (and IIRC is even checked in hidtest now), and it can be theoretically removed with express purpose of forbidding this (which could in theory cause deadlocks), though I would see that as an overkill.
Solving the issue on MacOS would be a better solution, even if we don't see a use for this use case at the moment.
Would also be delighted to see this land, FWIW
I'll refine the suggested behavior a bit in scope of https://github.com/libusb/hidapi/pull/790 (this is documentation-only) - and will make corresponding changes to the code later.
Any update on hotplug? Is it going to make it into a release anytime soon? I have merged OpenRGB next into master, currently depending on our hidapi-hotplug fork to support hotplugging. I would prefer if there was an official release we could link to but I don't want to delay OpenRGB 1.0 much longer.
I'll spend some time on it right now, but no promices.
As of right now, using a fork for OpenRGB release is a good compromise, but as of the official support - I see too many issues (including documentation/claims) to safely call it "ready for release".
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.