sync: make const_new methods always available (#5885)

Since MSRV is bumped to 1.63, `Mutex::new` is now usable in const context. Also use `assert!` in const function to ensure correctness instead of silently truncating the value and remove cfg `tokio_no_const_mutex_new`. Signed-off-by: Jiahao XU <[email protected]>
tokio-rs · Jul 28, 2023 · efe3ab6 · efe3ab6
1 parent fb08591
commit efe3ab6
Show file tree

Hide file tree

Showing 12 changed files with 213 additions and 245 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -442,7 +442,6 @@ jobs:
 
           # Run a platform without AtomicU64 and no const Mutex::new
           - target: armv5te-unknown-linux-gnueabi
-            rustflags: --cfg tokio_no_const_mutex_new
     steps:
       - uses: actions/checkout@v3
       - name: Install Rust stable
@@ -485,7 +484,6 @@ jobs:
 
           # Run a platform without AtomicU64 and no const Mutex::new
           - target: armv5te-unknown-linux-gnueabi
-            rustflags: --cfg tokio_no_const_mutex_new
     steps:
       - uses: actions/checkout@v3
       - name: Install Rust stable
@@ -568,10 +566,6 @@ jobs:
 
       # https://github.com/tokio-rs/tokio/pull/5356
       # https://github.com/tokio-rs/tokio/issues/5373
-      - name: Check without const_mutex_new
-        run: cargo hack check -p tokio --feature-powerset --depth 2 --keep-going
-        env:
-          RUSTFLAGS: --cfg tokio_unstable --cfg tokio_taskdump -Dwarnings --cfg tokio_no_atomic_u64 --cfg tokio_no_const_mutex_new
       - name: Check with const_mutex_new
         run: cargo hack check -p tokio --feature-powerset --depth 2 --keep-going
         env:

diff --git a/tokio/src/loom/std/mutex.rs b/tokio/src/loom/std/mutex.rs
@@ -13,7 +13,6 @@ impl<T> Mutex<T> {
     }
 
     #[inline]
-    #[cfg(not(tokio_no_const_mutex_new))]
     pub(crate) const fn const_new(t: T) -> Mutex<T> {
         Mutex(sync::Mutex::new(t))
     }

diff --git a/tokio/src/macros/cfg.rs b/tokio/src/macros/cfg.rs
@@ -541,13 +541,7 @@ macro_rules! cfg_not_has_atomic_u64 {
 macro_rules! cfg_has_const_mutex_new {
     ($($item:item)*) => {
         $(
-            #[cfg(all(
-                not(all(loom, test)),
-                any(
-                    feature = "parking_lot",
-                    not(tokio_no_const_mutex_new)
-                )
-            ))]
+            #[cfg(not(all(loom, test)))]
             $item
         )*
     }
@@ -556,13 +550,7 @@ macro_rules! cfg_has_const_mutex_new {
 macro_rules! cfg_not_has_const_mutex_new {
     ($($item:item)*) => {
         $(
-            #[cfg(not(all(
-                not(all(loom, test)),
-                any(
-                    feature = "parking_lot",
-                    not(tokio_no_const_mutex_new)
-                )
-            )))]
+            #[cfg(all(loom, test))]
             $item
         )*
     }

diff --git a/tokio/src/sync/batch_semaphore.rs b/tokio/src/sync/batch_semaphore.rs
@@ -178,14 +178,9 @@ impl Semaphore {
     /// Creates a new semaphore with the initial number of permits.
     ///
     /// Maximum number of permits on 32-bit platforms is `1<<29`.
-    ///
-    /// If the specified number of permits exceeds the maximum permit amount
-    /// Then the value will get clamped to the maximum number of permits.
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    pub(crate) const fn const_new(mut permits: usize) -> Self {
-        // NOTE: assertions and by extension panics are still being worked on: https://github.com/rust-lang/rust/issues/74925
-        // currently we just clamp the permit count when it exceeds the max
-        permits &= Self::MAX_PERMITS;
+    #[cfg(not(all(loom, test)))]
+    pub(crate) const fn const_new(permits: usize) -> Self {
+        assert!(permits <= Self::MAX_PERMITS);
 
         Self {
             permits: AtomicUsize::new(permits << Self::PERMIT_SHIFT),
@@ -198,6 +193,19 @@ impl Semaphore {
         }
     }
 
+    /// Creates a new closed semaphore with 0 permits.
+    pub(crate) fn new_closed() -> Self {
+        Self {
+            permits: AtomicUsize::new(Self::CLOSED),
+            waiters: Mutex::new(Waitlist {
+                queue: LinkedList::new(),
+                closed: true,
+            }),
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: tracing::Span::none(),
+        }
+    }
+
     /// Returns the current number of available permits.
     pub(crate) fn available_permits(&self) -> usize {
         self.permits.load(Acquire) >> Self::PERMIT_SHIFT

diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs
@@ -378,8 +378,7 @@ impl<T: ?Sized> Mutex<T> {
     ///
     /// static LOCK: Mutex<i32> = Mutex::const_new(5);
     /// ```
-    #[cfg(all(feature = "parking_lot", not(all(loom, test)),))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
+    #[cfg(not(all(loom, test)))]
     pub const fn const_new(t: T) -> Self
     where
         T: Sized,

diff --git a/tokio/src/sync/notify.rs b/tokio/src/sync/notify.rs
@@ -443,8 +443,7 @@ impl Notify {
     ///
     /// static NOTIFY: Notify = Notify::const_new();
     /// ```
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
+    #[cfg(not(all(loom, test)))]
     pub const fn const_new() -> Notify {
         Notify {
             state: AtomicUsize::new(0),

diff --git a/tokio/src/sync/once_cell.rs b/tokio/src/sync/once_cell.rs
@@ -114,12 +114,10 @@ impl<T> Drop for OnceCell<T> {
 
 impl<T> From<T> for OnceCell<T> {
     fn from(value: T) -> Self {
-        let semaphore = Semaphore::new(0);
-        semaphore.close();
         OnceCell {
             value_set: AtomicBool::new(true),
             value: UnsafeCell::new(MaybeUninit::new(value)),
-            semaphore,
+            semaphore: Semaphore::new_closed(),
         }
     }
 }
@@ -139,6 +137,10 @@ impl<T> OnceCell<T> {
     /// If the `Option` is `None`, this is equivalent to `OnceCell::new`.
     ///
     /// [`OnceCell::new`]: crate::sync::OnceCell::new
+    // Once https://github.com/rust-lang/rust/issues/73255 lands
+    // and tokio MSRV is bumped to the rustc version with it stablised,
+    // we can made this function available in const context,
+    // by creating `Semaphore::const_new_closed`.
     pub fn new_with(value: Option<T>) -> Self {
         if let Some(v) = value {
             OnceCell::from(v)
@@ -171,8 +173,7 @@ impl<T> OnceCell<T> {
     ///     assert_eq!(*result, 2);
     /// }
     /// ```
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
+    #[cfg(not(all(loom, test)))]
     pub const fn const_new() -> Self {
         OnceCell {
             value_set: AtomicBool::new(false),

diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs
@@ -334,8 +334,7 @@ impl<T: ?Sized> RwLock<T> {
     ///
     /// static LOCK: RwLock<i32> = RwLock::const_new(5);
     /// ```
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
+    #[cfg(not(all(loom, test)))]
     pub const fn const_new(value: T) -> RwLock<T>
     where
         T: Sized,
@@ -359,13 +358,13 @@ impl<T: ?Sized> RwLock<T> {
     ///
     /// static LOCK: RwLock<i32> = RwLock::const_with_max_readers(5, 1024);
     /// ```
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
-    pub const fn const_with_max_readers(value: T, mut max_reads: u32) -> RwLock<T>
+    #[cfg(not(all(loom, test)))]
+    pub const fn const_with_max_readers(value: T, max_reads: u32) -> RwLock<T>
     where
         T: Sized,
     {
-        max_reads &= MAX_READS;
+        assert!(max_reads <= MAX_READS);
+
         RwLock {
             mr: max_reads,
             c: UnsafeCell::new(value),

diff --git a/tokio/src/sync/semaphore.rs b/tokio/src/sync/semaphore.rs
@@ -172,20 +172,22 @@ impl Semaphore {
     ///
     /// static SEM: Semaphore = Semaphore::const_new(10);
     /// ```
-    ///
-    #[cfg(all(feature = "parking_lot", not(all(loom, test))))]
-    #[cfg_attr(docsrs, doc(cfg(feature = "parking_lot")))]
+    #[cfg(not(all(loom, test)))]
     pub const fn const_new(permits: usize) -> Self {
-        #[cfg(all(tokio_unstable, feature = "tracing"))]
-        return Self {
+        Self {
             ll_sem: ll::Semaphore::const_new(permits),
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
             resource_span: tracing::Span::none(),
-        };
+        }
+    }
 
-        #[cfg(any(not(tokio_unstable), not(feature = "tracing")))]
-        return Self {
-            ll_sem: ll::Semaphore::const_new(permits),
-        };
+    /// Creates a new closed semaphore with 0 permits.
+    pub(crate) fn new_closed() -> Self {
+        Self {
+            ll_sem: ll::Semaphore::new_closed(),
+            #[cfg(all(tokio_unstable, feature = "tracing"))]
+            resource_span: tracing::Span::none(),
+        }
     }
 
     /// Returns the current number of available permits.

diff --git a/tokio/src/util/mod.rs b/tokio/src/util/mod.rs
@@ -5,12 +5,7 @@ cfg_io_driver! {
 #[cfg(feature = "rt")]
 pub(crate) mod atomic_cell;
 
-#[cfg(any(
-    feature = "rt",
-    feature = "signal",
-    feature = "process",
-    tokio_no_const_mutex_new,
-))]
+#[cfg(any(feature = "rt", feature = "signal", feature = "process"))]
 pub(crate) mod once_cell;
 
 #[cfg(any(