Merge #913

913: Remove dependency on once_cell and work around windows-gnu LTO issue r=taiki-e a=taiki-e

Replace `once_cell` with our own `OnceLock` that based on unstable `std::sync::OnceLock` due to MSRV issue[^1].

This also fixes #856 due to windows-gnu LTO issue.

[^1]: [The current readme says crossbeam support at least 6 months old compilers](https://github.com/crossbeam-rs/crossbeam/blob/e54f01e7a7e3913407e817ce3e578a76b6392ef1/README.md#compatibility), but that is a minimum guarantee and is [actually more conservative](#877 (comment)).

Co-authored-by: Taiki Endo <te316e89@gmail.com>

.github/workflows/ci.yml

@@ -64,11 +64,6 @@ jobs:
       - name: Install cross
         uses: taiki-e/install-action@cross
         if: matrix.target != ''
-      # TODO: remove dependency on once_cell or bump MSRV
-      - name: Downgrade dependencies on MSRV
-        run: |
-          cargo update -p once_cell --precise 1.14.0
-        if: matrix.rust == '1.38'
       - name: Test
         run: ./ci/test.sh
 
@@ -90,11 +85,6 @@ jobs:
           toolchain: ${{ matrix.rust }}
       - name: Install cargo-hack
         uses: taiki-e/install-action@cargo-hack
-      # TODO: remove dependency on once_cell or bump MSRV
-      - name: Downgrade dependencies on MSRV
-        run: |
-          cargo update -p once_cell --precise 1.14.0
-        if: matrix.rust == '1.38'
       - name: Check features
         run: ./ci/check-features.sh
 

crossbeam-epoch/Cargo.toml

@@ -19,7 +19,7 @@ default = ["std"]
 
 # Enable to use APIs that require `std`.
 # This is enabled by default.
-std = ["alloc", "crossbeam-utils/std", "once_cell"]
+std = ["alloc", "crossbeam-utils/std"]
 
 # Enable to use APIs that require `alloc`.
 # This is enabled by default and also enabled if the `std` feature is enabled.
@@ -39,7 +39,6 @@ autocfg = "1"
 [dependencies]
 cfg-if = "1"
 memoffset = "0.6"
-once_cell = { version = "1", optional = true }
 scopeguard = { version = "1.1", default-features = false }
 
 # Enable the use of loom for concurrency testing.

crossbeam-epoch/src/default.rs

@@ -8,22 +8,30 @@ use crate::collector::{Collector, LocalHandle};
 use crate::guard::Guard;
 use crate::primitive::thread_local;
 #[cfg(not(crossbeam_loom))]
-use once_cell::sync::Lazy;
+use crate::sync::once_lock::OnceLock;
 
-/// The global data for the default garbage collector.
-#[cfg(not(crossbeam_loom))]
-static COLLECTOR: Lazy<Collector> = Lazy::new(Collector::new);
-// FIXME: loom does not currently provide the equivalent of Lazy:
-// https://github.com/tokio-rs/loom/issues/263
-#[cfg(crossbeam_loom)]
-loom::lazy_static! {
-    /// The global data for the default garbage collector.
-    static ref COLLECTOR: Collector = Collector::new();
+fn collector() -> &'static Collector {
+    #[cfg(not(crossbeam_loom))]
+    {
+        /// The global data for the default garbage collector.
+        static COLLECTOR: OnceLock<Collector> = OnceLock::new();
+        COLLECTOR.get_or_init(Collector::new)
+    }
+    // FIXME: loom does not currently provide the equivalent of Lazy:
+    // https://github.com/tokio-rs/loom/issues/263
+    #[cfg(crossbeam_loom)]
+    {
+        loom::lazy_static! {
+            /// The global data for the default garbage collector.
+            static ref COLLECTOR: Collector = Collector::new();
+        }
+        &COLLECTOR
+    }
 }
 
 thread_local! {
     /// The per-thread participant for the default garbage collector.
-    static HANDLE: LocalHandle = COLLECTOR.register();
+    static HANDLE: LocalHandle = collector().register();
 }
 
 /// Pins the current thread.
@@ -40,7 +48,7 @@ pub fn is_pinned() -> bool {
 
 /// Returns the default global collector.
 pub fn default_collector() -> &'static Collector {
-    &COLLECTOR
+    collector()
 }
 
 #[inline]
@@ -50,7 +58,7 @@ where
 {
     HANDLE
         .try_with(|h| f(h))
-        .unwrap_or_else(|_| f(&COLLECTOR.register()))
+        .unwrap_or_else(|_| f(&collector().register()))
 }
 
 #[cfg(all(test, not(crossbeam_loom)))]

crossbeam-epoch/src/lib.rs

@@ -108,7 +108,7 @@ mod primitive {
         // https://github.com/tokio-rs/loom#handling-loom-api-differences
         impl<T> UnsafeCell<T> {
             #[inline]
-            pub(crate) fn new(data: T) -> UnsafeCell<T> {
+            pub(crate) const fn new(data: T) -> UnsafeCell<T> {
                 UnsafeCell(::core::cell::UnsafeCell::new(data))
             }
 

crossbeam-epoch/src/sync/mod.rs

@@ -1,4 +1,7 @@
 //! Synchronization primitives.
 
 pub(crate) mod list;
+#[cfg(feature = "std")]
+#[cfg(not(crossbeam_loom))]
+pub(crate) mod once_lock;
 pub(crate) mod queue;

crossbeam-epoch/src/sync/once_lock.rs

@@ -0,0 +1 @@
+../../../crossbeam-utils/src/sync/once_lock.rs

crossbeam-utils/Cargo.toml

@@ -19,11 +19,10 @@ default = ["std"]
 
 # Enable to use APIs that require `std`.
 # This is enabled by default.
-std = ["once_cell"]
+std = []
 
 [dependencies]
 cfg-if = "1"
-once_cell = { version = "1", optional = true }
 
 # Enable the use of loom for concurrency testing.
 #

crossbeam-utils/src/sync/mod.rs

@@ -4,6 +4,8 @@
 //! * [`ShardedLock`], a sharded reader-writer lock with fast concurrent reads.
 //! * [`WaitGroup`], for synchronizing the beginning or end of some computation.
 
+#[cfg(not(crossbeam_loom))]
+mod once_lock;
 mod parker;
 #[cfg(not(crossbeam_loom))]
 mod sharded_lock;

crossbeam-utils/src/sync/once_lock.rs

@@ -0,0 +1,103 @@
+// Based on unstable std::sync::OnceLock.
+//
+// Source: https://github.com/rust-lang/rust/blob/8e9c93df464b7ada3fc7a1c8ccddd9dcb24ee0a0/library/std/src/sync/once_lock.rs
+
+use core::cell::UnsafeCell;
+use core::mem::MaybeUninit;
+use core::sync::atomic::{AtomicBool, Ordering};
+use std::sync::Once;
+
+pub(crate) struct OnceLock<T> {
+    once: Once,
+    // Once::is_completed requires Rust 1.43, so use this to track of whether they have been initialized.
+    is_initialized: AtomicBool,
+    value: UnsafeCell<MaybeUninit<T>>,
+    // Unlike std::sync::OnceLock, we don't need PhantomData here because
+    // we don't use #[may_dangle].
+}
+
+unsafe impl<T: Sync + Send> Sync for OnceLock<T> {}
+unsafe impl<T: Send> Send for OnceLock<T> {}
+
+impl<T> OnceLock<T> {
+    /// Creates a new empty cell.
+    #[must_use]
+    pub(crate) const fn new() -> Self {
+        Self {
+            once: Once::new(),
+            is_initialized: AtomicBool::new(false),
+            value: UnsafeCell::new(MaybeUninit::uninit()),
+        }
+    }
+
+    /// Gets the contents of the cell, initializing it with `f` if the cell
+    /// was empty.
+    ///
+    /// Many threads may call `get_or_init` concurrently with different
+    /// initializing functions, but it is guaranteed that only one function
+    /// will be executed.
+    ///
+    /// # Panics
+    ///
+    /// If `f` panics, the panic is propagated to the caller, and the cell
+    /// remains uninitialized.
+    ///
+    /// It is an error to reentrantly initialize the cell from `f`. The
+    /// exact outcome is unspecified. Current implementation deadlocks, but
+    /// this may be changed to a panic in the future.
+    pub(crate) fn get_or_init<F>(&self, f: F) -> &T
+    where
+        F: FnOnce() -> T,
+    {
+        // Fast path check
+        if self.is_initialized() {
+            // SAFETY: The inner value has been initialized
+            return unsafe { self.get_unchecked() };
+        }
+        self.initialize(f);
+
+        debug_assert!(self.is_initialized());
+
+        // SAFETY: The inner value has been initialized
+        unsafe { self.get_unchecked() }
+    }
+
+    #[inline]
+    fn is_initialized(&self) -> bool {
+        self.is_initialized.load(Ordering::Acquire)
+    }
+
+    #[cold]
+    fn initialize<F>(&self, f: F)
+    where
+        F: FnOnce() -> T,
+    {
+        let slot = self.value.get().cast::<T>();
+        let is_initialized = &self.is_initialized;
+
+        self.once.call_once(|| {
+            let value = f();
+            unsafe {
+                slot.write(value);
+            }
+            is_initialized.store(true, Ordering::Release);
+        });
+    }
+
+    /// # Safety
+    ///
+    /// The value must be initialized
+    unsafe fn get_unchecked(&self) -> &T {
+        debug_assert!(self.is_initialized());
+        &*self.value.get().cast::<T>()
+    }
+}
+
+impl<T> Drop for OnceLock<T> {
+    fn drop(&mut self) {
+        if self.is_initialized() {
+            // SAFETY: The inner value has been initialized
+            unsafe { self.value.get().cast::<T>().drop_in_place() };
+        }
+    }
+}

crossbeam-utils/src/sync/sharded_lock.rs

@@ -9,8 +9,8 @@ use std::sync::{LockResult, PoisonError, TryLockError, TryLockResult};
 use std::sync::{Mutex, RwLock, RwLockReadGuard, RwLockWriteGuard};
 use std::thread::{self, ThreadId};
 
+use crate::sync::once_lock::OnceLock;
 use crate::CachePadded;
-use once_cell::sync::Lazy;
 
 /// The number of shards per sharded lock. Must be a power of two.
 const NUM_SHARDS: usize = 8;
@@ -583,13 +583,17 @@ struct ThreadIndices {
     next_index: usize,
 }
 
-static THREAD_INDICES: Lazy<Mutex<ThreadIndices>> = Lazy::new(|| {
-    Mutex::new(ThreadIndices {
-        mapping: HashMap::new(),
-        free_list: Vec::new(),
-        next_index: 0,
-    })
-});
+fn thread_indices() -> &'static Mutex<ThreadIndices> {
+    static THREAD_INDICES: OnceLock<Mutex<ThreadIndices>> = OnceLock::new();
+    fn init() -> Mutex<ThreadIndices> {
+        Mutex::new(ThreadIndices {
+            mapping: HashMap::new(),
+            free_list: Vec::new(),
+            next_index: 0,
+        })
+    }
+    THREAD_INDICES.get_or_init(init)
+}
 
 /// A registration of a thread with an index.
 ///
@@ -601,7 +605,7 @@ struct Registration {
 
 impl Drop for Registration {
     fn drop(&mut self) {
-        let mut indices = THREAD_INDICES.lock().unwrap();
+        let mut indices = thread_indices().lock().unwrap();
         indices.mapping.remove(&self.thread_id);
         indices.free_list.push(self.index);
     }
@@ -610,7 +614,7 @@ impl Drop for Registration {
 thread_local! {
     static REGISTRATION: Registration = {
         let thread_id = thread::current().id();
-        let mut indices = THREAD_INDICES.lock().unwrap();
+        let mut indices = thread_indices().lock().unwrap();
 
         let index = match indices.free_list.pop() {
             Some(i) => i,