test(util): update loom to v0.5.4

The new Loom version 0.5.4 has new APIs (see tokio-rs/loom#219) that
make it possible to correctly simulate things that hand out `UnsafeCell`
pointers where the mutable/immutable access outlives a closure. This
makes it possible to have more correct tests for things like our `Mutex`
implementation.

util/Cargo.toml

@@ -13,7 +13,7 @@ alloc = []
 tracing = { git = "https://github.com/tokio-rs/tracing", default_features = false, features = ["attributes"] }
 
 [dev-dependencies]
-loom = "0.4"
+loom = "0.5.4"
 proptest = "1"
 tracing-subscriber = { git = "https://github.com/tokio-rs/tracing" }
 tracing = { git = "https://github.com/tokio-rs/tracing", default_features = false, features = ["attributes", "std"] }

util/src/cell.rs

@@ -1,43 +1,34 @@
-pub use self::causal::*;
-#[cfg(test)]
-mod causal {
-    // Clippy is trying to help us not hurt ourselves, but this is being renamed
-    // as an API compat stopgap b/c i haven't updated our stuff to the latest
-    // loom APIs...
-    #![allow(clippy::unsafe_removed_from_name)]
-    pub use loom::cell::UnsafeCell as CausalCell;
+pub use self::unsafe_cell::*;
+#[cfg(loom)]
+mod unsafe_cell {
+    pub use loom::cell::{ConstPtr, MutPtr, UnsafeCell};
 }
 
-#[cfg(not(test))]
-mod causal {
-    use core::cell::UnsafeCell;
+#[cfg(not(loom))]
+mod unsafe_cell {
+    use core::cell;
 
-    /// CausalCell ensures access to the inner value are valid under the Rust memory
+    /// `UnsafeCell` ensures access to the inner value are valid under the Rust memory
     /// model.
     ///
     /// When not running under `loom`, this is identical to an `UnsafeCell`.
     #[derive(Debug)]
-    pub struct CausalCell<T> {
-        data: UnsafeCell<T>,
+    pub struct UnsafeCell<T> {
+        data: cell::UnsafeCell<T>,
     }
 
-    unsafe impl<T> Sync for CausalCell<T> {}
+    #[derive(Debug)]
+    pub(crate) struct ConstPtr<T: ?Sized>(*const T);
 
-    /// Deferred causal cell check.
-    ///
-    /// When not running under `loom`, this does nothing.
     #[derive(Debug)]
-    #[must_use]
-    pub struct CausalCheck {
-        _p: (),
-    }
+    pub(crate) struct MutPtr<T: ?Sized>(*mut T);
 
-    impl<T> CausalCell<T> {
+    impl<T> UnsafeCell<T> {
         /// Construct a new instance of `CausalCell` which will wrap the specified
         /// value.
-        pub const fn new(data: T) -> CausalCell<T> {
+        pub const fn new(data: T) -> Self {
             Self {
-                data: UnsafeCell::new(data),
+                data: cell::UnsafeCell::new(data),
             }
         }
 
@@ -82,5 +73,53 @@ mod causal {
         {
             f(self.data.get())
         }
+
+        #[inline(always)]
+        pub(crate) fn get(&self) -> ConstPtr<T> {
+            ConstPtr(self.data.get())
+        }
+
+        #[inline(always)]
+        pub(crate) fn get_mut(&self) -> MutPtr<T> {
+            MutPtr(self.data.get())
+        }
+    }
+
+    // === impl MutPtr ===
+
+    impl<T: ?Sized> MutPtr<T> {
+        // Clippy knows that it's Bad and Wrong to construct a mutable reference
+        // from an immutable one...but this function is intended to simulate a raw
+        // pointer, so we have to do that here.
+        #[allow(clippy::mut_from_ref)]
+        #[inline(always)]
+        pub(crate) unsafe fn deref(&self) -> &mut T {
+            &mut *self.0
+        }
+
+        #[inline(always)]
+        pub fn with<F, R>(&self, f: F) -> R
+        where
+            F: FnOnce(*mut T) -> R,
+        {
+            f(self.0)
+        }
+    }
+
+    // === impl ConstPtr ===
+
+    impl<T: ?Sized> ConstPtr<T> {
+        #[inline(always)]
+        pub(crate) unsafe fn deref(&self) -> &T {
+            &*self.0
+        }
+
+        #[inline(always)]
+        pub fn with<F, R>(&self, f: F) -> R
+        where
+            F: FnOnce(*const T) -> R,
+        {
+            f(self.0)
+        }
     }
 }

util/src/sync/spin.rs

@@ -1,5 +1,5 @@
 use super::atomic::{AtomicBool, Ordering::*};
-use crate::cell::CausalCell;
+use crate::cell::{MutPtr, UnsafeCell};
 use core::{
     fmt,
     ops::{Deref, DerefMut},
@@ -9,27 +9,28 @@ use core::{
 #[derive(Debug)]
 pub struct Mutex<T> {
     locked: AtomicBool,
-    data: CausalCell<T>,
+    data: UnsafeCell<T>,
 }
 
 pub struct MutexGuard<'a, T> {
-    mutex: &'a Mutex<T>,
+    ptr: MutPtr<T>,
+    locked: &'a AtomicBool,
 }
 
 impl<T> Mutex<T> {
-    #[cfg(not(test))]
+    #[cfg(not(loom))]
     pub const fn new(data: T) -> Self {
         Self {
             locked: AtomicBool::new(false),
-            data: CausalCell::new(data),
+            data: UnsafeCell::new(data),
         }
     }
 
-    #[cfg(test)]
+    #[cfg(loom)]
     pub fn new(data: T) -> Self {
         Self {
             locked: AtomicBool::new(false),
-            data: CausalCell::new(data),
+            data: UnsafeCell::new(data),
         }
     }
 
@@ -39,7 +40,10 @@ impl<T> Mutex<T> {
             .compare_exchange(false, true, Acquire, Acquire)
             .is_ok()
         {
-            Some(MutexGuard { mutex: self })
+            Some(MutexGuard {
+                ptr: self.data.get_mut(),
+                locked: &self.locked,
+            })
         } else {
             None
         }
@@ -56,7 +60,11 @@ impl<T> Mutex<T> {
                 boff.spin();
             }
         }
-        MutexGuard { mutex: self }
+
+        MutexGuard {
+            ptr: self.data.get_mut(),
+            locked: &self.locked,
+        }
     }
 
     /// Forcibly unlock the mutex.
@@ -85,14 +93,24 @@ unsafe impl<T: Send> Sync for Mutex<T> {}
 
 impl<'a, T> Deref for MutexGuard<'a, T> {
     type Target = T;
+    #[inline]
     fn deref(&self) -> &Self::Target {
-        self.mutex.data.with(|ptr| unsafe { &*ptr })
+        unsafe {
+            // Safety: we are holding the lock, so it is okay to dereference the
+            // mut pointer.
+            &*self.ptr.deref()
+        }
     }
 }
 
 impl<'a, T> DerefMut for MutexGuard<'a, T> {
+    #[inline]
     fn deref_mut(&mut self) -> &mut Self::Target {
-        self.mutex.data.with_mut(|ptr| unsafe { &mut *ptr })
+        unsafe {
+            // Safety: we are holding the lock, so it is okay to dereference the
+            // mut pointer.
+            self.ptr.deref()
+        }
     }
 }
 
@@ -102,7 +120,7 @@ where
 {
     #[inline]
     fn as_ref(&self) -> &R {
-        self.mutex.data.with(|ptr| unsafe { &*ptr }.as_ref())
+        self.deref().as_ref()
     }
 }
 
@@ -112,15 +130,13 @@ where
 {
     #[inline]
     fn as_mut(&mut self) -> &mut R {
-        self.mutex
-            .data
-            .with_mut(|ptr| unsafe { &mut *ptr }.as_mut())
+        self.deref_mut().as_mut()
     }
 }
 
 impl<'a, T> Drop for MutexGuard<'a, T> {
     fn drop(&mut self) {
-        self.mutex.locked.store(false, Release);
+        self.locked.store(false, Release);
     }
 }
 
@@ -136,7 +152,7 @@ impl<'a, T: fmt::Display> fmt::Display for MutexGuard<'a, T> {
     }
 }
 
-#[cfg(test)]
+#[cfg(all(test, loom))]
 mod tests {
     use loom::thread;
     use std::prelude::v1::*;