Add `MaybeUninit::assume_init_read` Safety constraint: it can easily break `!Send` invariant

[gksato]

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 !Send type to another thread (for example, by sharing &MaybeUninit<T> across threads and calling assume_init_read there). Breaking Send or Sync guarantees may lead to undefined behavior such as data races.

As an additional note, API authors whose safe abstraction relies on assume_init_read should consider re-implementing Send/Sync with a stricter bound, e.g. requiring T: Send, because the default auto-trait derivation from MaybeUninit<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.

[Noratrieb]

The root of this problem is the assumption of unsafe impl<T: Sync> Send for &T, an extremely fundamental implementation for Rusts thread safety guarantees. The notable assumption that it makes is that if you have a &T, you don't move out of it. This is an extremely reasonable assumption, but also a very non-local assumption, that means that everything that could allow you to do that needs to consider it.

This whole method seems a bit funky because it moves out of a &T, but given that it exists, documenting the effects on ownership and considering Send more clearly seems good.

[theemathas]

Maybe if we had a time machine, the assume_init_read method could have had a &mut self receiver instead...

[Amanieu]

We discussed this in the @rust-lang/libs meeting and we're happy to have this text added to the docs. The precise wording can be refined as part of the PR review.

[gksato]

Nice, I've been rewording it, I guess I'll be able to send a PR on this weekend!