ruabmbua / hidapi-rs

Rust bindings for the hidapi C library
MIT License
172 stars 81 forks source link

Made HidApi struct thread-safe #39

Closed AlexCouch closed 5 years ago

AlexCouch commented 5 years ago

I ran into a problem where I had different threads trying to reference my program's HidApi instance. However, HidApi was not thread-safe due to the _lock field being a non-async reference counted HidApiLock Rc<HidApiLock>. I changed this to be Arc<HidApiLock>, that way multiple threads can reference the hidapi instance. I provided a really crappy example of it working.

I know the underlying lower level implications of this that this could allow dependents to write to a HidDevice from multiple threads but I think that we cannot control that due to the fact that this relies on the C implementation of hidapi as the backend. I see that there is currently plans for isolating this library into its own pure rust implementation and I believe that would make way for a more thread-safe implementation of this library unless we can do some kind of hacky write access block if the device is currently already being written to in the write binding. I haven't completely explored this yet but I honestly just needed a simple way of allowing multiple threads to reference my hidapi instance. My project's implementation is simply just a single thread for reading and a single thread for writing. This is the fix that I came up with.

I know the example is very bad and I couldn't demonstrate multiple device opens in parallel but it seems to be just fine. And that's another thing that could be dangerous: multiple threads opening a device concurrently. The point is, I made this library just a tiny bit more thread-safe while also introducing some possible hazards. But it's a fix for now I guess.

ruabmbua commented 5 years ago

Hi!

Yes you are indeed correct, that just making the lock threadsafe does not give you any real thread safety.

I am however not quite sure, if signal11/hidapi is or is not threadsafe.

Someone else had problems with accessing hidapi with multiple threads, and I suggested using a channel based API: https://github.com/ruabmbua/hidapi-rs/issues/16

You will have to read through the whole thread + comments, somewhere in the middle of it the channel API is mentioned.

There is also https://docs.rs/hidapi/0.5.2/hidapi/struct.HidDevice.html#method.set_blocking_mode. You can set it to false, to activate non blocking mode. If everything you want is fast reaction time, then you can set up a event loop, where you handle your protocol. Usually you would use something like poll, epoll, etc..., but that is not really supported by this crate.

Example (pseudo rust code):


fn io_loop() {
    let mut device = ...;
    device.set_blocking_mode(false);

    loop {
         let rdata = device.read(...); // Will return immediately
         handle_read_data(rdata);
         if check_if_want_write() {
             device.write(...);
         }
         // Optionally put something like thread::sleep(...); here to not burn all your cpu time
    }
} 
ruabmbua commented 5 years ago

There is also HidDevice::read_timeout(). It will guarantee to return, when the given timeout is over.