Skip to content

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Sep 6, 2025

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.

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.
@mcuee mcuee added the macOS label Sep 6, 2025
@seanm
Copy link
Contributor Author

seanm commented Sep 17, 2025

@hjelmn does this look right to you?

@seanm
Copy link
Contributor Author

seanm commented Sep 19, 2025

@osy could you take a look at this?

@osy
Copy link
Contributor

osy commented Sep 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
Copy link
Contributor Author

seanm commented Sep 19, 2025

@osy Yes reading. This change is as a result of a -Wthread-safety compiler warning. According to existing comments:

	/* lock protects claimed_interfaces */
	usbi_mutex_t lock;
	unsigned long claimed_interfaces;

claimed_interfaces should be protected by lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants