rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

RefInit: RAII wrapper for initialized MaybeUninit references #338

Open ajwock opened 5 months ago

ajwock commented 5 months ago

Proposal

Problem statement

Values initialized by MaybeUninit ref APIs and MaybeUninit slice APIs will have their destructors leaked unless the user manually drops them.

Motivating examples or use cases

https://github.com/rust-lang/rust/issues/80376 MaybeUninit::write_slice_cloned() makes it very easy to accidentally leak

https://users.rust-lang.org/t/is-there-a-way-to-copy-t-into-mut-maybeuninit-t-without-unsafe/51301/2

This is also an issue that has come up in discussion with this ACP: https://github.com/rust-lang/libs-team/issues/156

A similar solution to the below is also used in MaybeUninit::clone_from_slice to Drop elements that are already initialized in the case of a panic: https://github.com/rust-lang/rust/blob/02438348b9c26c0d657cc7b990fd1f45a8c0c736/library/core/src/mem/maybe_uninit.rs#L1128

Solution sketch

Introducing: RefInit. An RAII wrapper that wraps the now-initialized reference, and drops the elements inside of it when it is dropped.

A rough sketch demonstrating what a solution might look like for the current set of APIs (including pending ones from #156 )

struct RefInit<'a, T: ?Sized> {
    val: &'a mut T,
}

impl<'a, T: ?Sized> Drop for RefInit<'a, T> {
    fn drop(&mut self) {
        // Safety:  RefInit is only yielded from calls on MaybeUninit slices,
        // so Drop will not be run again when the source of the reference goes out of
        // scope.
        unsafe {
            std::ptr::drop_in_place(self.val);
        }
    }
}

impl<'a, T: ?Sized> Deref for RefInit<'a, T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        self.val
    }
}

impl<'a, T: ?Sized> DerefMut for RefInit<'a, T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.val
    }
}

impl<'a, T> RefInit<'a, T> {
    pub unsafe fn assume_init_mut(uninit_ref: &'a mut MaybeUninit<T>) -> RefInit<'a, T> {
        RefInit {
            val: unsafe { MaybeUninit::assume_init_mut(uninit_ref) },
        }
    }

    pub fn write(uninit_ref: &'a mut MaybeUninit<T>, val: T) -> RefInit<'a, T> {
        RefInit {
            val: MaybeUninit::write(uninit_ref, val),
        }
    }

}

impl<'a, T: ?Sized> RefInit<'a, T> {
    pub fn leak(self) -> &'a mut T {
        let ret = unsafe {
            &mut *(self.val as *mut T)
        };
        let _ = std::mem::ManuallyDrop::new(self);
        ret
    }
}

impl<'a, T> RefInit<'a, [T]> {
    pub unsafe fn slice_assume_init_mut(slice: &'a mut [MaybeUninit<T>]) -> RefInit<'a, [T]> {
        RefInit {
            val: unsafe { MaybeUninit::slice_assume_init_mut(slice) },
        }
    }

    pub fn copy_from_slice(slice: &'a mut [MaybeUninit<T>], src: &[T]) -> RefInit<'a, [T]>
    where
        T: Copy,
    {
        RefInit {
            val: MaybeUninit::copy_from_slice(slice, src),
        }
    }

    pub fn clone_from_slice(slice: &'a mut [MaybeUninit<T>], src: &[T]) -> RefInit<'a, [T]>
    where
        T: Clone,
    {
        RefInit {
            val: MaybeUninit::clone_from_slice(slice, src),
        }
    }

    pub fn fill(slice: &'a mut [MaybeUninit<T>], value: T) -> RefInit<'a, [T]>
    where
        T: Clone,
    {
        RefInit {
            val: MaybeUninit::fill(slice, value),
        }
    }

    pub fn fill_from<I>(
        slice: &'a mut [MaybeUninit<T>],
        it: I,
    ) -> (RefInit<'a, [T]>, &'a mut [MaybeUninit<T>])
    where
        I: IntoIterator<Item = T>,
    {
        let (initialized, remainder) = MaybeUninit::fill_from(slice, it);
        (RefInit { val: initialized }, remainder)
    }

    pub fn fill_with<F>(slice: &'a mut [MaybeUninit<T>], f: F) -> RefInit<'a, [T]>
    where
        F: FnMut() -> T,
    {
        RefInit {
            val: MaybeUninit::fill_with(slice, f),
        }
    }
}

Alternatives

Alternatives are rather lacking here. Manually dropping is an alternative, albeit not a safe alternative.

Linking relevant APIs

https://github.com/rust-lang/rust/issues/63567

https://github.com/rust-lang/rust/issues/79995

https://github.com/rust-lang/libs-team/issues/156

ajwock commented 5 months ago

One more thought.

Would it make sense to change most of the APIs listed above in MaybeUninit to return RefInit rather than having this be an option on the side?

All such APIs in their current state will cause a destructor leak without future use of unsafe, and with RefInit's leak function the old behavior can be preserved if desired while also making it more explicit that destructors will be leaked unless other measures are taken.

For assume_init_mut / slice_assume_init_mut there should be a separate method for the InitRef version though, because in those you aren't necessarily initializing elements on the spot.