Remove unneeded Send/Sync bounds on Arc, Mutex, RwLock and sync::mpsc #23176
Conversation
The requirement `T: Send + Sync` only matters if the `Arc` crosses thread boundaries, and that is adequately controlled by the impls of `Send`/`Sync` for `Arc` itself. If `T` doesn't satisfy the bounds, then the `Arc` cannot cross thread boundaries and so everything is still safe (`Arc` just acts like an expensive `Rc`).
The requirements `T: Send` and `T: Send + Sync` for `Mutex` and `RwLock` respectively only matter if those types are shared/sent across thread boundaries, and that is adequately controlled by the impls of `Send`/`Sync` for them. If `T` doesn't satisfy the bounds, then the types cannot cross thread boundaries and so everything is still safe (the two types just act like an expensive `RefCell`).
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
The requirements `T: Send` only matter if the channel crosses thread boundaries i.e. the `Sender` or `Reciever` are sent across thread boundaries, and which is adequately controlled by the impls of `Send` for them. If `T` doesn't satisfy the bounds, then the types cannot cross thread boundaries and so everything is still safe (the pair of types collectively behave like a `Rc<RefCell<VecDeque>>`, or something of that nature).
|
Alternatively, we could tighten up these types and make them require the relevant bounds everywhere (closing the |
|
I've seen this definitely assist in r+ from me, but would like a second opinion from @brson or @aturon as well. |
|
It seems like there's a policy that is unclear here, and these bounds may be tighter than necessary on purpose. I do have a vague recollection that we wanted bounds on types at some point for reasons similar to what @alexcrichton was saying. |
|
I am inclined not to take this step, and instead to further tighten up the use of Like @alexcrichton, I'm worried about error messages and clarity here. Further, I'm not convinced about the generic programming angle. In a case where you're generic enough to not be able to send an Finally, as you point out, keeping the bounds tighter is the more conservative option. |
These types are purely designed for concurrent code, and, at the moment, are restricted to be used in those situations. This solidifies that goal by imposing a strict restriction on the generic types from the start, i.e. in the definition itself. This is the opposite to rust-lang#23176 which relaxes the bounds entirely (it is backwards compatible to switch to that more flexible approach). Unfortunately the message-passing primitives in std (the return value from a thread, and sync::mpsc) aren't great about how they work with mutability and sharing, and so require hacky error-prone `unsafe impl`s. However, they are purely implementation details: the interface isn't affected by having to make that internal change, and clean-ups to the code should be able to remove the hacks.
|
Tighter bounds implemented in #23954. |
|
@bors: r+ |
|
📌 Commit 0f6b43a has been approved by |
|
⌛ Testing commit 0f6b43a with merge c271382... |
|
💔 Test failed - auto-mac-64-opt |
These types had many instances of
T: Send + Syncin places they were not necessary.In fact, the only place they are required is when considering under what circumstances can
Arc<T>(etc.) be sent/shared across thread boundaries, i.e. the only place it matters is theunsafe impls ofSendandSync. It is perfectly safe for anArc<T>to stay on and be used on a single thread ifTdoesn't satisfy the necessary bounds. It just acts like a (expensive)Rc<T>. The added flexibility may be convenient in generic code. (Similar reasoning applies toMutex,RwLockandsync::mpsc. See commit messages for some more details.)This also fixes a hole, where
Arc::newhas no bounds onT, butDrop for Arc<T>requiresT: Send + Sync(meaning one could theoretically create anArctype which has no destructor, although the compiler mishandles this case anyway).