Add hotplug support

[mcuee]

One of the main missing feature of hidapi is the hotplug support.

[mcuee]

Technically there are some Windows codes are out there to support hotplug. For Linux you can consider libudev. For macOS there are similar codes as well. libusb has hotplug support under Linux and macOS but not Windows (complicated due to libusb Windows implementation).

[mcuee]

For Windows, not so sure pywinusb can shed some light.
https://github.com/rene-aguirre/pywinusb

Or the codes here.
http://janaxelson.com/hidpage.htm

I think Hidsharp also supports hotplug.
https://github.com/IntergatedCircuits/HidSharp

[Youw]

I'd say that hotplug support for each platform is pretty much straight-forward (I've done it for macOS and Windows several years ago in a propriatary project)
same is true for linux (udev) and libusb backends

it just has to be implemented

[mcuee]

#299 seems to work pretty well under Windows. The API function names are now similar to libusb and I think they are fine.

[807183087]

Hi,sir. I found a little error about nullptr in https://github.com/DJm00n/hidapi/blob/device_notify/windows/hid.c#L1036

[807183087]

I noticed, the plugged devices before my testing program startup also can be callback.
I think, hotplug just processes the hot plug events, but not existed devices.

[Youw]

I think, hotplug just processes the hot plug events, but not existed devices.

Once hotplug is registered - it has to report all existing devices.

The likely alternative you probably want to have is:

1. enumerate existing devices;
2. register callback for new devices;

Or the other way around (first register callback, then enumerate existing devices).

In either case you're having a race condition if the device is plugged at the moment you're registering a callback (and that is happening more often than you could imagine).
You either miss a device or have it reported twice.

The only 100% reliable way to use device hotplug events - is to use only hotplug callback events to get a list of all devices (existing and new).
And not use things like hid_enumerate at all.

[807183087]

yes.
registe callback like this will not receive existed devices:
hid_hotplug_register_callback(0, 0, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED| HID_API_HOTPLUG_EVENT_DEVICE_LEFT, 0/*HID_API_HOTPLUG_ENUMERATE*/, hotplug_callback, this, &hotplug_handle);

[gekart]

Just to understand this issue: Are you saying that hidapi is not USB hotplug ready? Then how to deal with disconnecting devices (on MacOS this might cause app and even system crashes)?

[Youw]

Then how to deal with disconnecting devices

As of right now - only by using OS-specific API in your application directly.

(on MacOS this might cause app and even system crashes)?

HIDAPI handles those internally (e.g.: 1) - no crashes are expected.

[gekart]

I am using a library (ZWO EFW SDK) that in turn uses hidapi. I get crashes constantly when plugging and unplugging the device wildly:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00007ff807fb363e CoreFoundation`_CFAssertMismatchedTypeID + 110
    frame #1: 0x00007ff807ea83ca CoreFoundation`CFRunLoopStop + 129
    frame #2: 0x00007ff80a6d5e5e IOKit`__IOHIDDeviceNotification + 195
    frame #3: 0x00007ff80a69bcda IOKit`IODispatchCalloutFromCFMessage + 318
    frame #4: 0x00007ff807ea826b CoreFoundation`__CFMachPortPerform + 288
    frame #5: 0x00007ff807e7baf1 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 41
    frame #6: 0x00007ff807e7b9d3 CoreFoundation`__CFRunLoopDoSource1 + 615
    frame #7: 0x00007ff807e7a047 CoreFoundation`__CFRunLoopRun + 2419
    frame #8: 0x00007ff807e79014 CoreFoundation`CFRunLoopRunSpecific + 562
    frame #9: 0x00000001000060a2 test_console`hid_enumerate + 178
    frame #10: 0x0000000100004a6b test_console`EFWGetNum + 59
    frame #11: 0x000000010000378d test_console`crashMeC() + 13
    frame #12: 0x00000001000037e9 test_console`main + 9
    frame #13: 0x000000020008952e dyld`start + 462

More context at https://bbs.astronomy-imaging-camera.com/d/15289-efwgetnum-on-macos-not-thread-safe/5 (That thread is full of assumptions, however, sorry...)

[Youw]

1. What version of HIDAPI is used here?
2. Can you confirm that hid_enumerate is not tried to be used from multiple threads concurently?

[Youw]

Oh, additionally: not only hid_enumerate - none of hid_init/hid_enumerate/hid_open/hid_open_path/hid_close/hid_exit are thread-safe on macOS - and it has nothing to do with hotplug support.

[gekart]

Hmm. That simple C ten-liner that I have linked above results in multiple threads:

(lldb) thread list
Process 8318 stopped
  thread #1: tid = 0x94125, 0x00007ff807d7b2be libsystem_kernel.dylib`__semwait_signal + 10, queue = 'com.apple.main-thread'
  thread #2: tid = 0x9415e, 0x00007ff807d7a05a libsystem_kernel.dylib`__workq_kernreturn + 10
* thread #3: tid = 0x9416a, 0x00007ff807fb363e CoreFoundation`_CFAssertMismatchedTypeID + 110, stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  thread #4: tid = 0x9415f, 0x00007ff807d7a05a libsystem_kernel.dylib`__workq_kernreturn + 10
(lldb) bt
* thread #3, stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00007ff807fb363e CoreFoundation`_CFAssertMismatchedTypeID + 110
    frame #1: 0x00007ff807ea83ca CoreFoundation`CFRunLoopStop + 129
    frame #2: 0x00007ff80a6d5e5e IOKit`__IOHIDDeviceNotification + 195
    frame #3: 0x00007ff80a69bcda IOKit`IODispatchCalloutFromCFMessage + 318
    frame #4: 0x00007ff807ea826b CoreFoundation`__CFMachPortPerform + 288
    frame #5: 0x00007ff807e7baf1 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__ + 41
    frame #6: 0x00007ff807e7b9d3 CoreFoundation`__CFRunLoopDoSource1 + 615
    frame #7: 0x00007ff807e7a047 CoreFoundation`__CFRunLoopRun + 2419
    frame #8: 0x00007ff807e79014 CoreFoundation`CFRunLoopRunSpecific + 562
    frame #9: 0x0000000100007032 test_console`read_thread + 258
    frame #10: 0x00007ff807db54e1 libsystem_pthread.dylib`_pthread_start + 125
    frame #11: 0x00007ff807db0f6b libsystem_pthread.dylib`thread_start + 15

so I assume they are using threads there. I don't think they are accessing hidapi from multiple threads.

If this doesn't have anything to do with hotplugging, maybe I should open another issue. At least it happens while hot plugging...

[Youw]

If this doesn't have anything to do with hotplugging, maybe I should open another issue.

Yes please.
At least this thread is about having an additional callbacks to report plugged/unplugged devices, and not about dealing with unplugged devices in runtime.
Thanks.

[gekart]

No need for another issue: I have fixed the problem by replacing the ar packaged version of hidapi within that SDK with a fresh recompile from GitHub. Companies keep their software closed to hide ugly truths... Thank you for your help!

[k1-801]

I noticed that the current branch only supports hotplug on windows, so I added a prototype implementation for libusb in #645. It's a pretty straightforward one, taking huge chunks from Windows implementation. Please review. has a couple of missing spots, will fill them in in the next few hours.
UPD: Fixed all the main issues I could find myself, requesting comments.

[k1-801]

Also noticed that the issue #538 has been stale for 4 months. #646 should resolve that, but I have not tested it yet (will report when I get to a windows machine to test it).

[k1-801]

Added a hidraw implementation (#647 ) that copies a lot from the libusb one (I specifically tried to move the major differences into separate functions, so that the general callback list management is the same exact code). Did not run any tests yet, that will have to wait until at least the weekends.

I don't have a Mac to build or test the last one... It's not going to stop me, but it's gonna take a bit longer than it took for the others.

UPD: Ran some tests on hidraw version, fixed some errors, it works fine now. I still have to ask someone to double-check everything, up to and including code-style.
Couldn't get libusb one to work, libusb itself does not read any events even under sudo. Probably something wrong with udev rules on my system.

[k1-801]

I have made a mock-up for a MacOS implementation of the hotplug feature in #653 . Even if it can not be used as-is, it will simplify the work to get this completed.

I beieve this completes the list of platforms that have to have the hotplug support for the hotplug feature to be included into the next release of hidapi. However, this one I am the least sure of, as it took me a whole two weeks and I stil couldn't set up a proper buid environment for it, so I can not even check if it builds (github CI might become my only way to test it), let alone check if it works.

[Youw]

@k1-801 please excuse me for not looking into your PRs. Really appreciate all the work you've done.

Got extremely busy lately.
I promice to get back to it as soon as get enought time for it.

[k1-801]

@Youw sorry to disturb you, but is there any chance of getting a review?
Didn't want to be rude, but it's been 3 months 😅

[Youw]

Thant's for the reminder. I actually do have a few hours today. Will take a look.

[k1-801]

Somehow - and I am not sure how - I got the MacOS implementation to a buildable state, with all the basic blocks of the implementation in place. All that's left now is to somehow make sure it works - and then give it a full review.

Additionally, I noticed that clang produced a few warnings in the pre-existing code that might require their own issue:

[k1-801]

I think it's best to rebase the connection-callback branch onto the fresh master. There are some conflicts now due to the updates that happened in the last 5 months.

[mcuee]

I think it's best to rebase the connection-callback branch onto the fresh master. There are some conflicts now due to the updates that happened in the last 5 months.

Good point.

@Youw
Please help here. Thanks.

[k1-801]

@Youw is there anything else that needs to be done or anything else needed from my side to get the feature reviewed, merged and then considered to be included in the next release?

I understand that there's nothing worse than releasing an untested feature, but so far we've gotten very good results on 3 out of the 4 backends, libusb being the only tricky one.

[k1-801]

The libusb PR now works as well.
No driver detachment is needed to receive events.
There are no untested platforms left.

[k1-801]

As for the practical applications of this feature, at this moment the project: https://gitlab.com/CalcProgrammer1/OpenRGB/-/merge_requests/1963 is waiting for this feature to be released.

[k1-801]

I just found out that there is actually now support for a netbsd backend in hidapi, and the connection-callback branch doesn't have stub callback management functions for that backend. Making a PR now.
UPD: #668

Unfortunately, I couldn't find any information on how to get device notifications on netbsd.

[k1-801]

A small concern here: calling any Register or Unregister functions from within a callback may (and probably will) cause a deadlock. On Windows, Critical Sections seem to keep an internal counter that allows the same thread to enter on multiple times, while in pthread, unless PTHREAD_MUTEX_RECURSIVE is specified, it will cause a deadlock.

Additionally, just unlocking the mutex before calling the callback (or making it recursive) will allow the user to remove elements in such a way that it disturbs the execution flow, which can already happen on Windows.

Bad scenarios:

• Removing the element right before the one currently being processed will invalidate the current pointer (as it will be pointing to an area that has just been free'd), and even if use-after-free situation doesn't break things, returning a value from the callback that deletes the callback will process the list incorrectly
• Removing the current element will lead to exactly the same use-after-free scenario
• Removing all elements may cause even more severe side-effects, as removing the last element triggers a cleanup sequence
• If the next pointer is stored in advance (which it isn't in current implementation, but should be warned against in the future), adding a new element while the last one is being processed will only cause the new one to not be processed while removing an element right after the current one will cause unpredictable behavior.

There is a way to circumvent this, by adding a flag to each callback that indicates that it should be removed later when it is considered safe, which is set by the Unregister function if the mutex is already locked at the moment of the call. It seems that, as long as we know for a fact that the mutex is locked by the same thread, it should be safe to read and navigate the list of callbacks and even set flags to it, and the list will not be changed (the mutex is still locked and will not be unloced until we leave the callback and the callback processing part). As for the Register function, I see no problem with additional callbacks being registered from a callback. However, this approach still requires to somehow know for a fact that the mutex is locked by the same thread we are currently in.

Another possible resolution would be to warn against the use of Register and Unregister functions inside a callback, however, there are scenarios where the user code could make use of that (a widely used example would be the scenario where the unplug callback is only registered after a device was connected).