Upgrade parking_lot to 0.10 version and use MaybeUninit for it

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.4.0
+
+- upgrade `parking_lot` to `0.10` (note that this bumps MSRV with `parking_lot` feature enabled to `1.36.0`).
+
 ## 1.3.1
 
 - remove unnecessary `F: fmt::Debug` bound from `impl fmt::Debug for Lazy<T, F>`.

Cargo.toml

@@ -19,7 +19,7 @@ exclude = ["*.png", "*.svg", "/Cargo.lock.min", "/.travis.yml", "/run-miri-tests
 # Uses parking_lot to implement once_cell::sync::OnceCell.
 # This makes not speed difference, but makes each OnceCell<T>
 # for up to two bytes smaller, depending on the size of the T.
-parking_lot = { version = "0.9.0", optional = true, default_features = false }
+parking_lot = { version = "0.10.0", optional = true, default_features = false }
 
 [dev-dependencies]
 lazy_static = "1.0.0"

examples/bench_acquire.rs

@@ -1,6 +1,6 @@
-/// Benchmark the overhead that the synchronization of `OnceCell::get` causes.
-/// We do some other operations that write to memory to get an imprecise but somewhat realistic
-/// measurement.
+//! Benchmark the overhead that the synchronization of `OnceCell::get` causes.
+//! We do some other operations that write to memory to get an imprecise but somewhat realistic
+//! measurement.
 
 use once_cell::sync::OnceCell;
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -29,7 +29,7 @@ fn thread_main(i: usize) {
     let mut data = [i; 128];
     let mut accum = 0usize;
     for _ in 0..N_ROUNDS {
-        let _value = CELL.get_or_init(|| i+1);
+        let _value = CELL.get_or_init(|| i + 1);
         let k = OTHER.fetch_add(data[accum & 0x7F] as usize, Ordering::Relaxed);
         for j in data.iter_mut() {
             *j = (*j).wrapping_add(accum);

examples/test_synchronization.rs

@@ -1,11 +1,11 @@
-/// Test if the OnceCell properly synchronizes.
-/// Needs to be run in release mode.
-///
-/// We create a `Vec` with `N_ROUNDS` of `OnceCell`s. All threads will walk the `Vec`, and race to
-/// be the first one to initialize a cell.
-/// Every thread adds the results of the cells it sees to an accumulator, which is compared at the
-/// end.
-/// All threads should end up with the same result.
+//! Test if the OnceCell properly synchronizes.
+//! Needs to be run in release mode.
+//!
+//! We create a `Vec` with `N_ROUNDS` of `OnceCell`s. All threads will walk the `Vec`, and race to
+//! be the first one to initialize a cell.
+//! Every thread adds the results of the cells it sees to an accumulator, which is compared at the
+//! end.
+//! All threads should end up with the same result.
 
 use once_cell::sync::OnceCell;
 

src/imp_pl.rs

@@ -1,6 +1,8 @@
 use std::{
     cell::UnsafeCell,
+    mem::{self, MaybeUninit},
     panic::{RefUnwindSafe, UnwindSafe},
+    ptr,
     sync::atomic::{AtomicBool, Ordering},
 };
 
@@ -9,7 +11,7 @@ use parking_lot::{lock_api::RawMutex as _RawMutex, RawMutex};
 pub(crate) struct OnceCell<T> {
     mutex: Mutex,
     is_initialized: AtomicBool,
-    pub(crate) value: UnsafeCell<Option<T>>,
+    value: UnsafeCell<MaybeUninit<T>>,
 }
 
 // Why do we need `T: Send`?
@@ -28,7 +30,21 @@ impl<T> OnceCell<T> {
         OnceCell {
             mutex: Mutex::new(),
             is_initialized: AtomicBool::new(false),
-            value: UnsafeCell::new(None),
+            value: UnsafeCell::new(MaybeUninit::uninit()),
+        }
+    }
+
+    fn as_ptr(&self) -> *const T {
+        unsafe {
+            let slot: &MaybeUninit<T> = &*self.value.get();
+            slot.as_ptr()
+        }
+    }
+
+    fn as_mut_ptr(&self) -> *mut T {
+        unsafe {
+            let slot: &mut MaybeUninit<T> = &mut *self.value.get();
+            slot.as_mut_ptr()
         }
     }
 
@@ -56,13 +72,62 @@ impl<T> OnceCell<T> {
             // - 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<T> = unsafe { &mut *self.value.get() };
-            debug_assert!(slot.is_none());
-            *slot = Some(value);
+            // Safe b/c we have a unique access and no panic may happen
+            // until the cell is marked as initialized.
+            unsafe { self.value.get().write(MaybeUninit::new(value)) };
             self.is_initialized.store(true, Ordering::Release);
         }
         Ok(())
     }
+
+    /// Get the reference to the underlying value, without checking if the cell
+    /// is initialized.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure that the cell is in initialized state, and that
+    /// the contents are acquired by (synchronized to) this thread.
+    pub(crate) unsafe fn get_unchecked(&self) -> &T {
+        debug_assert!(self.is_initialized());
+        &*self.as_ptr()
+    }
+
+    /// Gets the mutable reference to the underlying value.
+    /// Returns `None` if the cell is empty.
+    pub(crate) fn get_mut(&mut self) -> Option<&mut T> {
+        if self.is_initialized() {
+            // Safe b/c we have a unique access and value is initialized.
+            Some(unsafe { &mut *self.as_mut_ptr() })
+        } else {
+            None
+        }
+    }
+
+    /// Consumes this `OnceCell`, returning the wrapped value.
+    /// Returns `None` if the cell was empty.
+    pub(crate) fn into_inner(self) -> Option<T> {
+        if !self.is_initialized() {
+            return None;
+        }
+
+        // Safe b/c we have a unique access and value is initialized.
+        let value: T = unsafe { ptr::read(self.as_ptr()) };
+
+        // It's OK to `mem::forget` without dropping, because both `self.mutex`
+        // and `self.is_initialized` are not heap-allocated.
+        mem::forget(self);
+
+        Some(value)
+    }
+}
+
+impl<T> Drop for OnceCell<T> {
+    fn drop(&mut self) {
+        if self.is_initialized() {
+            // Safe b/c we have a unique access and value is initialized.
+            unsafe { ptr::drop_in_place(self.as_mut_ptr()) };
+        }
+    }
 }
 
 /// Wrapper around parking_lot's `RawMutex` which has `const fn` new.
@@ -92,9 +157,8 @@ impl Drop for MutexGuard<'_> {
 }
 
 #[test]
-#[cfg(target_pointer_width = "64")]
 fn test_size() {
     use std::mem::size_of;
 
-    assert_eq!(size_of::<OnceCell<u32>>(), 3 * size_of::<u32>());
+    assert_eq!(size_of::<OnceCell<bool>>(), 2 * size_of::<bool>() + size_of::<u8>());
 }

src/imp_std.rs

@@ -5,6 +5,7 @@
 
 use std::{
     cell::{Cell, UnsafeCell},
+    hint::unreachable_unchecked,
     marker::PhantomData,
     panic::{RefUnwindSafe, UnwindSafe},
     sync::atomic::{AtomicBool, AtomicUsize, Ordering},
@@ -21,7 +22,7 @@ pub(crate) struct OnceCell<T> {
     // 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<Option<T>>,
+    value: UnsafeCell<Option<T>>,
 }
 
 // Why do we need `T: Send`?
@@ -106,6 +107,43 @@ impl<T> OnceCell<T> {
         });
         res
     }
+
+    /// Get the reference to the underlying value, without checking if the cell
+    /// is initialized.
+    ///
+    /// # Safety
+    ///
+    /// Caller must ensure that the cell is in initialized state, and that
+    /// the contents are acquired by (synchronized to) this thread.
+    pub(crate) unsafe fn get_unchecked(&self) -> &T {
+        debug_assert!(self.is_initialized());
+        let slot: &Option<T> = &*self.value.get();
+        match slot {
+            Some(value) => value,
+            // This unsafe does improve performance, see `examples/bench`.
+            None => {
+                debug_assert!(false);
+                unreachable_unchecked()
+            }
+        }
+    }
+
+    /// Gets the mutable reference to the underlying value.
+    /// Returns `None` if the cell is empty.
+    pub(crate) fn get_mut(&mut self) -> Option<&mut T> {
+        // Safe b/c we have a unique access.
+        unsafe { &mut *self.value.get() }.as_mut()
+    }
+
+    /// Consumes this `OnceCell`, returning the wrapped value.
+    /// Returns `None` if the cell was empty.
+    #[inline]
+    pub(crate) fn into_inner(self) -> Option<T> {
+        // 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<T>`.
+        self.value.into_inner()
+    }
 }
 
 // Corresponds to `std::sync::Once::call_inner`

src/lib.rs

@@ -170,7 +170,8 @@ equivalents with `RefCell` and `Mutex`.
 
 # Minimum Supported `rustc` Version
 
-This crate's minimum supported `rustc` version is `1.31.1`.
+This crate's minimum supported `rustc` version is `1.31.1` (or `1.36.0` with
+`parking_lot` feature enabled).
 
 If only `std` feature is enabled, MSRV will be updated conservatively.
 When using other features, like `parking_lot`, MSRV might be updated more frequently, up to the latest stable.
@@ -570,7 +571,6 @@ pub mod sync {
     use std::{
         cell::Cell,
         fmt,
-        hint::unreachable_unchecked,
         ops::{Deref, DerefMut},
         panic::RefUnwindSafe,
     };
@@ -663,7 +663,7 @@ pub mod sync {
         /// method never blocks.
         pub fn get(&self) -> Option<&T> {
             if self.0.is_initialized() {
-                // Safe b/c checked is_initialize
+                // Safe b/c value is initialized.
                 Some(unsafe { self.get_unchecked() })
             } else {
                 None
@@ -674,28 +674,18 @@ 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()
+            self.0.get_mut()
         }
 
         /// Get the reference to the underlying value, without checking if the
         /// cell is initialized.
         ///
-        /// Safety:
+        /// # Safety
         ///
         /// Caller must ensure that the cell is in initialized state, and that
         /// 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<T> = &*self.0.value.get();
-            match slot {
-                Some(value) => value,
-                // This unsafe does improve performance, see `examples/bench`.
-                None => {
-                    debug_assert!(false);
-                    unreachable_unchecked()
-                }
-            }
+            self.0.get_unchecked()
         }
 
         /// Sets the contents of this cell to `value`.
@@ -802,7 +792,7 @@ pub mod sync {
             }
             self.0.initialize(f)?;
 
-            // Safe b/c called initialize
+            // Safe b/c value is initialized.
             debug_assert!(self.0.is_initialized());
             Ok(unsafe { self.get_unchecked() })
         }
@@ -823,9 +813,7 @@ pub mod sync {
         /// assert_eq!(cell.into_inner(), Some("hello".to_string()));
         /// ```
         pub fn into_inner(self) -> Option<T> {
-            // 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<T>`.
-            self.0.value.into_inner()
+            self.0.into_inner()
         }
     }