libs-team
libs-team copied to clipboard
RefInit: RAII wrapper for initialized MaybeUninit references
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
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.