From 8a261a2b342d85d11c67a8d4dd3408778bd0f5b7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 12 Sep 2020 13:40:50 +0200 Subject: [PATCH 1/2] Simplify SyncOnceCell's `take` and `drop`. --- library/std/src/lazy.rs | 39 +++++++++++++-------------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/library/std/src/lazy.rs b/library/std/src/lazy.rs index 091e2091fb095..e0095e64faf31 100644 --- a/library/std/src/lazy.rs +++ b/library/std/src/lazy.rs @@ -7,7 +7,7 @@ use crate::{ cell::{Cell, UnsafeCell}, fmt, marker::PhantomData, - mem::{self, MaybeUninit}, + mem::MaybeUninit, ops::{Deref, Drop}, panic::{RefUnwindSafe, UnwindSafe}, sync::Once, @@ -316,13 +316,7 @@ impl SyncOnceCell { /// ``` #[unstable(feature = "once_cell", issue = "74465")] pub fn into_inner(mut self) -> Option { - // SAFETY: Safe because we immediately free `self` without dropping - let inner = unsafe { self.take_inner() }; - - // Don't drop this `SyncOnceCell`. We just moved out one of the fields, but didn't set - // the state to uninitialized. - mem::forget(self); - inner + self.take() } /// Takes the value out of this `SyncOnceCell`, moving it back to an uninitialized state. @@ -348,22 +342,12 @@ impl SyncOnceCell { /// ``` #[unstable(feature = "once_cell", issue = "74465")] pub fn take(&mut self) -> Option { - mem::take(self).into_inner() - } - - /// Takes the wrapped value out of a `SyncOnceCell`. - /// Afterwards the cell is no longer initialized. - /// - /// Safety: The cell must now be free'd WITHOUT dropping. No other usages of the cell - /// are valid. Only used by `into_inner` and `drop`. - unsafe fn take_inner(&mut self) -> Option { - // The mutable reference guarantees there are no other threads that can observe us - // taking out the wrapped value. - // Right after this function `self` is supposed to be freed, so it makes little sense - // to atomically set the state to uninitialized. if self.is_initialized() { - let value = mem::replace(&mut self.value, UnsafeCell::new(MaybeUninit::uninit())); - Some(value.into_inner().assume_init()) + self.once = Once::new(); + // SAFETY: `self.value` is initialized and contains a valid `T`. + // `self.once` is reset, so `is_initialized()` will be false again + // which prevents the value from being read twice. + unsafe { Some((&mut *self.value.get()).assume_init_read()) } } else { None } @@ -416,9 +400,12 @@ impl SyncOnceCell { unsafe impl<#[may_dangle] T> Drop for SyncOnceCell { fn drop(&mut self) { - // SAFETY: The cell is being dropped, so it can't be accessed again. - // We also don't touch the `T`, which validates our usage of #[may_dangle]. - unsafe { self.take_inner() }; + if self.is_initialized() { + // Safety: The cell is initialized and being dropped, so it can't + // be accessed again. We also don't touch the `T` other than + // dropping it, which validates our usage of #[may_dangle]. + unsafe { (&mut *self.value.get()).assume_init_drop() }; + } } } From aa68aaa8e1b6c667987b71a83c332f1ce0988e54 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 12 Sep 2020 17:11:47 +0200 Subject: [PATCH 2/2] Mark Once::new as #[inline]. Without this, it was not inlined in SyncOnceCell::into_inner(), causing unecessary checks and dead code. --- library/std/src/sync/once.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 8fed369bffc27..29ae338cb2ec7 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -191,6 +191,7 @@ struct WaiterQueue<'a> { impl Once { /// Creates a new `Once` value. + #[inline] #[stable(feature = "once_new", since = "1.2.0")] #[rustc_const_stable(feature = "const_once_new", since = "1.32.0")] pub const fn new() -> Once {