Skip to content

Commit

Permalink
Rollup merge of rust-lang#76640 - fusion-engineering-forks:synconcece…
Browse files Browse the repository at this point in the history
…ll-drop, r=matklad

Simplify SyncOnceCell's `take` and `drop`.

Prevents copies by using `assume_init_read` and `assume_init_drop`.
  • Loading branch information
RalfJung authored Sep 13, 2020
2 parents 945189a + aa68aaa commit 70fb791
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 26 deletions.
39 changes: 13 additions & 26 deletions library/std/src/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -316,13 +316,7 @@ impl<T> SyncOnceCell<T> {
/// ```
#[unstable(feature = "once_cell", issue = "74465")]
pub fn into_inner(mut self) -> Option<T> {
// 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.
Expand All @@ -348,22 +342,12 @@ impl<T> SyncOnceCell<T> {
/// ```
#[unstable(feature = "once_cell", issue = "74465")]
pub fn take(&mut self) -> Option<T> {
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<T> {
// 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
}
Expand Down Expand Up @@ -416,9 +400,12 @@ impl<T> SyncOnceCell<T> {

unsafe impl<#[may_dangle] T> Drop for SyncOnceCell<T> {
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() };
}
}
}

Expand Down
1 change: 1 addition & 0 deletions library/std/src/sync/once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 70fb791

Please sign in to comment.