- Notifications
You must be signed in to change notification settings - Fork2k
darwin: add missing mutex lock/unlock for claimed_interfaces#1695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
Existing code comments say thatlibusb_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_callbackand none seem to hold the lock already.Found with -Wthread-safety.
seanm commentedSep 17, 2025
@hjelmn does this look right to you? |
seanm commentedSep 19, 2025
@osy could you take a look at this? |
osy commentedSep 19, 2025
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. |
seanm commentedSep 19, 2025
@osy Yes reading. This change is as a result of a -Wthread-safety compiler warning. According to existing comments:
|
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.