Add MaybeUninit::assume_init_read Safety constraint: it can easily break !Send invariant #148083
Description
Location (URL)
https://doc.rust-lang.org/std/mem/union.MaybeUninit.html#method.assume_init_read
Summary
MaybeUninit::<T>::assume_init_read, together with impl<T: Sync> Sync for MaybeUninit<T>, can be (mis)used to break the Send invariant: when T: Sync, one can share &MaybeUninit<T> across threads (since &MaybeUninit<T>: Send) and then assume_init_read a fresh owned T on another thread. If T: Sync + !Send, this constitutes a transfer of ownership of a !Send value between threads, which violates Send's safety contract and can lead to undefined behavior (such as data races). The current Safety section does not mention this class of misuse.
Concretely, consider the following code (Playground):
pub mod mover {
use core::mem::MaybeUninit;
pub struct Mover<'a, T: 'a>(&'a MaybeUninit<T>);
impl<'a, T: 'a> Mover<'a, T> {
/// Put a value in `Mover`.
pub fn new(t: T) -> Self {
Mover(Box::leak(Box::new(MaybeUninit::new(t))))
}
/// Take an inner value out.
pub fn into_inner(self) -> T {
unsafe {
// SAFETY: self.0 is initialized via new().
// self is destructed, so no duplication will occur.
// HOWEVER: this allows us to `!Send` move across threads.
self.0.assume_init_read()
}
}
}
}
use mover::Mover;
use std::thread;
// Compiles because `&MaybeUninit<T>` is `Send` when `T: Sync`.
// `Mover<'_, T>` is thus `Send` under the same condition.
fn horror<T, U, F: FnOnce(T) -> U>(t: T, f: F) -> thread::JoinHandle<U>
where
T: Sync + 'static,
U: Send + 'static,
F: Send + 'static,
{
let m: Mover<'static, T> = Mover::new(t);
thread::spawn(|| f(m.into_inner()))
}This does not violate the two explicit constraints in the current documentation (the value is initialized, and not duplicated), yet it breaks the Send invariant for T, sending T:Sync + !Send across threads. Violating Send/Sync invariants may cause undefined behavior such as data races.
Request: Extend the Safety section with wording like:
In addition, the caller must uphold the thread-safety auto-trait invariants of
T. In particular, do not use this function to move an instance of a!Sendtype to another thread (for example, by sharing&MaybeUninit<T>across threads and callingassume_init_readthere). BreakingSendorSyncguarantees may lead to undefined behavior such as data races.As an additional note, API authors whose safe abstraction relies on
assume_init_readshould consider re-implementingSend/Syncwith a stricter bound, e.g. requiringT: Send, because the default auto-trait derivation fromMaybeUninit<T>may be too permissive.
ptr::read can cause this kind of issue and doesn't document this danger, but raw pointers are neither Sync nor Send. MaybeUninit::assume_init_read can exploit this and we have impl<T: Sync> Sync for MaybeUninit<T>, so I believe we need a good documentation.
I can send a PR as soon as we agree that this is actually a problem and that an additional Safety invariant documentation is a good way to resolve this! I don't think anyone wants to resolve this by introducing unsafe impl<T: Send + Sync> Sync for MaybeUninit<T>, but I'd like to be sure.