Should more `DeviceHandle` methods take a shared `self` reference?
mxk opened this issue · 4 comments
libusb is generally thread-safe, and since DeviceHandle isn't reference-counted, it seems like more methods (e.g. reset, clear_halt, set_alternate_setting, etc.) should take &self rather than &mut self to enable shared usage via Arc<DeviceHandle<T>>. Thoughts?
You can share DeviceHandle with Arc even some of function take &mut self. I think it's good protection to call this function if you own DeviceHandle. No one prevent you share DeviceHandle with Arc and call write_bulk/read_bulk from multiply thread. But what happened if you read or write in one thread and try to reset or clear_halt in other. And that's why these functions take &mut self.
We can discuss how this should be handled.
I see your point, but these decisions were already made by libusb authors, so I would say that a wrapper shouldn't place additional restrictions. Rather, it should do whatever libusb would do if reset is called in parallel with other operations, assuming that such use is valid.
In my case, I wrote a wrapper around libusb's async transfer API, which required putting the DeviceHandle behind an Arc, but then needed access to some of the &mut self methods elsewhere. Worked around it with unsafe, but didn't see a good reason why that should be needed. I can also envision use cases where you actually do want to reset the device while other things are happening to implement watchdog-like functionality.
Ok. I am agree. I will try to make new update for library. But I think it will be at the end of October or begin November. It will be breaking changes.
Just take note libusb_reset() behavior is platform dependant. Under Windows it may be even driver dependant. libusb gives the freedom to the user and user can shoot themselves by calling libusb_reet() in a multithread application.
As for rusb, I think it does not really need to follow libusb in this aspect.