diff --git a/.travis.yml b/.travis.yml index 5c53053..c0abd10 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,6 +32,11 @@ matrix: script: - cargo build --no-default-features --test test + - rust: stable + script: + - cargo test --no-default-features --features "std maybe_uninit" + - cargo test --no-default-features --features "std maybe_uninit parking_lot" + - rust: 1.31.1 script: - mv Cargo.lock.min Cargo.lock diff --git a/Cargo.toml b/Cargo.toml index fa28791..2ab8349 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,8 @@ regex = "1.2.0" default = ["std"] # Enables `once_cell::sync` module. std = [] +# Use `MaybeUninit` in the `sync` module. +maybe_uninit = [] [[example]] name = "bench" diff --git a/src/imp_pl.rs b/src/imp_pl.rs index 3c5b1ec..69cd3cc 100644 --- a/src/imp_pl.rs +++ b/src/imp_pl.rs @@ -3,13 +3,14 @@ use std::{ panic::{RefUnwindSafe, UnwindSafe}, sync::atomic::{AtomicBool, Ordering}, }; +use crate::maybe_uninit::MaybeUninit; use parking_lot::{lock_api::RawMutex as _RawMutex, RawMutex}; pub(crate) struct OnceCell { mutex: Mutex, is_initialized: AtomicBool, - pub(crate) value: UnsafeCell>, + pub(crate) value: UnsafeCell>, } // Why do we need `T: Send`? @@ -28,7 +29,7 @@ impl OnceCell { OnceCell { mutex: Mutex::new(), is_initialized: AtomicBool::new(false), - value: UnsafeCell::new(None), + value: UnsafeCell::new(MaybeUninit::uninit()), } } @@ -56,9 +57,11 @@ impl OnceCell { // - finally, if it returns Ok, we store the value and store the flag with // `Release`, which synchronizes with `Acquire`s. let value = f()?; - let slot: &mut Option = unsafe { &mut *self.value.get() }; - debug_assert!(slot.is_none()); - *slot = Some(value); + unsafe { + let slot: &mut MaybeUninit = &mut *self.value.get(); + // FIXME: replace with `slot.as_mut_ptr().write(value)` + slot.write(value); + } self.is_initialized.store(true, Ordering::Release); } Ok(()) @@ -92,9 +95,8 @@ impl Drop for MutexGuard<'_> { } #[test] -#[cfg(target_pointer_width = "64")] fn test_size() { use std::mem::size_of; - assert_eq!(size_of::>(), 3 * size_of::()); + assert_eq!(size_of::>(), 3 * size_of::()); } diff --git a/src/imp_std.rs b/src/imp_std.rs index f5a6ea5..f76ad98 100644 --- a/src/imp_std.rs +++ b/src/imp_std.rs @@ -10,18 +10,14 @@ use std::{ sync::atomic::{AtomicBool, AtomicUsize, Ordering}, thread::{self, Thread}, }; +use crate::maybe_uninit::MaybeUninit; -#[derive(Debug)] pub(crate) struct OnceCell { // This `state` word is actually an encoded version of just a pointer to a // `Waiter`, so we add the `PhantomData` appropriately. state_and_queue: AtomicUsize, _marker: PhantomData<*mut Waiter>, - // FIXME: switch to `std::mem::MaybeUninit` once we are ready to bump MSRV - // that far. It was stabilized in 1.36.0, so, if you are reading this and - // it's higher than 1.46.0 outside, please send a PR! ;) (and do the same - // for `Lazy`, while we are at it). - pub(crate) value: UnsafeCell>, + pub(crate) value: UnsafeCell>, } // Why do we need `T: Send`? @@ -66,7 +62,7 @@ impl OnceCell { OnceCell { state_and_queue: AtomicUsize::new(INCOMPLETE), _marker: PhantomData, - value: UnsafeCell::new(None), + value: UnsafeCell::new(MaybeUninit::uninit()), } } @@ -95,7 +91,11 @@ impl OnceCell { let f = f.take().unwrap(); match f() { Ok(value) => { - unsafe { *slot.get() = Some(value) }; + unsafe { + let slot: &mut MaybeUninit = &mut *slot.get(); + // FIXME: replace with `slot.as_mut_ptr().write(value)` + slot.write(value); + } true } Err(e) => { @@ -315,10 +315,9 @@ mod tests { } #[test] - #[cfg(target_pointer_width = "64")] fn test_size() { use std::mem::size_of; - assert_eq!(size_of::>(), 4 * size_of::()); + assert_eq!(size_of::>(), 2 * size_of::()); } } diff --git a/src/lib.rs b/src/lib.rs index e7bd7f6..cd7cba5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -233,6 +233,8 @@ mod imp; #[path = "imp_std.rs"] mod imp; +mod maybe_uninit; + pub mod unsync { use core::{ cell::{Cell, UnsafeCell}, @@ -568,14 +570,15 @@ pub mod unsync { #[cfg(feature = "std")] pub mod sync { use std::{ - cell::Cell, + cell::{Cell, UnsafeCell}, fmt, - hint::unreachable_unchecked, + mem::{self, ManuallyDrop}, ops::{Deref, DerefMut}, panic::RefUnwindSafe, }; use crate::imp::OnceCell as Imp; + use crate::maybe_uninit::MaybeUninit; /// A thread-safe cell which can be written to only once. /// @@ -622,6 +625,12 @@ pub mod sync { } } + impl Drop for OnceCell { + fn drop(&mut self) { + self.take_inner(); + } + } + impl Clone for OnceCell { fn clone(&self) -> OnceCell { let res = OnceCell::new(); @@ -675,7 +684,11 @@ pub mod sync { /// Returns `None` if the cell is empty. pub fn get_mut(&mut self) -> Option<&mut T> { // Safe b/c we have a unique access. - unsafe { &mut *self.0.value.get() }.as_mut() + if self.0.is_initialized() { + Some(unsafe { &mut *(*self.0.value.get()).as_mut_ptr() }) + } else { + None + } } /// Get the reference to the underlying value, without checking if the @@ -687,15 +700,8 @@ pub mod sync { /// the contents are acquired by (synchronized to) this thread. pub unsafe fn get_unchecked(&self) -> &T { debug_assert!(self.0.is_initialized()); - let slot: &Option = &*self.0.value.get(); - match slot { - Some(value) => value, - // This unsafe does improve performance, see `examples/bench`. - None => { - debug_assert!(false); - unreachable_unchecked() - } - } + let slot: &MaybeUninit = &*self.0.value.get(); + &*slot.as_ptr() } /// Sets the contents of this cell to `value`. @@ -822,10 +828,28 @@ pub mod sync { /// cell.set("hello".to_string()).unwrap(); /// assert_eq!(cell.into_inner(), Some("hello".to_string())); /// ``` - pub fn into_inner(self) -> Option { - // Because `into_inner` takes `self` by value, the compiler statically verifies - // that it is not currently borrowed. So it is safe to move out `Option`. - self.0.value.into_inner() + pub fn into_inner(mut self) -> Option { + let res = self.take_inner(); + // Don't drop this `OnceCell`. We just moved out one of the fields, but didn't set + // the state to uninitialized. + ManuallyDrop::new(self); + res + } + + /// Takes the wrapped value out of a `OnceCell`. + /// Afterwards the cell is no longer initialized, and must be free'd WITHOUT dropping. + /// Only used by `into_inner` and `drop`. + 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.0.is_initialized() { + let value = mem::replace(&mut self.0.value, UnsafeCell::new(MaybeUninit::uninit())); + Some(unsafe { value.into_inner().assume_init() }) + } else { + None + } } } @@ -863,6 +887,7 @@ pub mod sync { /// ``` pub struct Lazy T> { cell: OnceCell, + // FIXME: replace with MaybeUninit init: Cell>, } diff --git a/src/maybe_uninit.rs b/src/maybe_uninit.rs new file mode 100644 index 0000000..c757058 --- /dev/null +++ b/src/maybe_uninit.rs @@ -0,0 +1,85 @@ +//! This module contains a re-export or vendored version of `core::mem::MaybeUninit` depending +//! on which Rust version it's compiled for. +//! +//! Remove this module and use `core::mem::MaybeUninit` directly when dropping support for <1.36 + +/// This is a terrible imitation of `core::mem::MaybeUninit` to support Rust older than 1.36. +/// Differences from the real deal: +/// - We drop the values contained in `MaybeUninit`, while that has to be done manually otherwise; +/// - We use more memory; +/// - `as_mut_ptr()` can't be used to initialize the `MaybeUninit`. + +#[cfg(feature = "maybe_uninit")] +pub struct MaybeUninit(core::mem::MaybeUninit); + +#[cfg(feature = "maybe_uninit")] +impl MaybeUninit { + #[inline] + pub const fn uninit() -> MaybeUninit { + MaybeUninit(core::mem::MaybeUninit::uninit()) + } + + #[inline] + pub fn as_ptr(&self) -> *const T { + self.0.as_ptr() + } + + #[inline] + pub fn as_mut_ptr(&mut self) -> *mut T { + self.0.as_mut_ptr() + } + + // Emulate `write`, which is only available on nightly. + #[inline] + pub unsafe fn write(&mut self, value: T) -> &mut T { + let slot = self.0.as_mut_ptr(); + slot.write(value); + &mut *slot + } + + #[inline] + pub unsafe fn assume_init(self) -> T { + self.0.assume_init() + } +} + + +#[cfg(not(feature = "maybe_uninit"))] +pub struct MaybeUninit(Option); + +#[cfg(not(feature = "maybe_uninit"))] +impl MaybeUninit { + #[inline] + pub const fn uninit() -> MaybeUninit { + MaybeUninit(None) + } + + #[inline] + pub fn as_ptr(&self) -> *const T { + match self.0.as_ref() { + Some(value) => value, + None => { + // This unsafe does improve performance, see `examples/bench`. + debug_assert!(false); + unsafe { core::hint::unreachable_unchecked() } + } + } + } + + #[inline] + pub fn as_mut_ptr(&mut self) -> *mut T { + self.0.as_mut().unwrap() + } + + // It would be better to use `as_mut_ptr().write()`, but that can't be emulated with `Option`. + #[inline] + pub unsafe fn write(&mut self, val: T) -> &mut T { + self.0 = Some(val); + self.0.as_mut().unwrap() + } + + #[inline] + pub unsafe fn assume_init(self) -> T { + self.0.unwrap() + } +}