darwin: add missing mutex lock/unlock for claimed_interfaces#1695
darwin: add missing mutex lock/unlock for claimed_interfaces#1695seanm wants to merge 1 commit intolibusb:masterfrom
Conversation
Existing code comments say that libusb_device_handle.claimed_interfaces is protected by libusb_device_handle.lock. The function ep_to_pipeRef() accesses claimed_interfaces but does not hold the lock. I examined all 9 callers: - darwin_alloc_streams - darwin_clear_halt - darwin_free_streams - darwin_abort_transfers - submit_bulk_transfer - submit_control_transfer - submit_iso_transfer - submit_stream_transfer - darwin_async_io_callback and none seem to hold the lock already. Found with -Wthread-safety.
|
@hjelmn does this look right to you? |
|
@osy could you take a look at this? |
|
This is a fix for a read race on claimed_interface (not write race) right? It looks fine to me but just a heads up, when I looked at the Darwin backend, I found a lot of read races and just reasoned that all of them would result in eventual errors that is similar to if the device was just hot unplugged. |
|
@osy Yes reading. This change is as a result of a -Wthread-safety compiler warning. According to existing comments:
|
|
I wouldn't be surprised if there are races with hot-unplug. Thanks for starting to take a look at this. My time is often 110% booked so I haven't had the time needed to do some of the housekeeping the darwin backend needs. Let me review this and make sure it isn't running afoul of something else. It is probably ok. |
|
This looks technically correct, the best kind of correct. |
Existing code comments say that
libusb_device_handle.claimed_interfaces is protected by libusb_device_handle.lock. The function ep_to_pipeRef() accesses claimed_interfaces but does not hold the lock.
I examined all 9 callers:
and none seem to hold the lock already.
Found with -Wthread-safety.