In the Rust code, these appear:
unsafe impl Send for Display {}
unsafe impl Sync for Display {}This is...not great. It essentially means any code entering Display will ignore thread-safety protections. For instance: two calls to set_mode might interleave, and get each other's responses.
- Python is moving towards free-threading, so this presents an issue in Python.
- Rust assumes that any method with a
&self receiver can be used from multiple threads safely; marking something Sync without it actually being thread-safe reintroduces C-style unsafety into Rust programs that are not themselves unsafe. This breaks a contract with Rust callers.
- C can leave a lot up to the caller, e.g. "these methods must only be called by a single thread", but this isn't documented.
It would be good to adhere to Rust's safety standards, so they can trickle back to the other APIs. Any use of unsafe should have a block explaining why it is in fact safe; in this case, adhering to the requirements that Sync and Send provide.
In this case -- I think the simplest solutions are either:
- Putting an internal mutex in
Display, around the HID device; this should allow removing the unsafe impls
- Having a lower-level
Display type without a mutex, with &mut self receivers--this would be a thread-unsafe object, so e.g. callers in C can use it without incurring the cost of the mutex-- and a ThreadSafeDisplay that wraps it in a mutex. The latter would be exported to Python.
In the Rust code, these appear:
This is...not great. It essentially means any code entering
Displaywill ignore thread-safety protections. For instance: two calls toset_modemight interleave, and get each other's responses.&selfreceiver can be used from multiple threads safely; marking somethingSyncwithout it actually being thread-safe reintroduces C-style unsafety into Rust programs that are not themselvesunsafe. This breaks a contract with Rust callers.It would be good to adhere to Rust's safety standards, so they can trickle back to the other APIs. Any use of
unsafeshould have a block explaining why it is in fact safe; in this case, adhering to the requirements thatSyncandSendprovide.In this case -- I think the simplest solutions are either:
Display, around the HID device; this should allow removing theunsafe implsDisplaytype without a mutex, with&mut selfreceivers--this would be a thread-unsafe object, so e.g. callers in C can use it without incurring the cost of the mutex-- and aThreadSafeDisplaythat wraps it in a mutex. The latter would be exported to Python.