fix downgrade (#329)

* fix downgrade

* add test for downgrade

* fix mapped refs to use ?Sized and introduce some additional tests for map

* more mappedref coverage

* fix drop order with temp result

Author: conradludgate

src/mapref/one.rs

@@ -30,7 +30,7 @@ impl<'a, K: Eq + Hash, V> Ref<'a, K, V> {
         (self.k, self.v)
     }
 
-    pub fn map<F, T>(self, f: F) -> MappedRef<'a, K, T>
+    pub fn map<F, T: ?Sized>(self, f: F) -> MappedRef<'a, K, T>
     where
         F: FnOnce(&V) -> &T,
     {
@@ -41,7 +41,7 @@ impl<'a, K: Eq + Hash, V> Ref<'a, K, V> {
         }
     }
 
-    pub fn try_map<F, T>(self, f: F) -> Result<MappedRef<'a, K, T>, Self>
+    pub fn try_map<F, T: ?Sized>(self, f: F) -> Result<MappedRef<'a, K, T>, Self>
     where
         F: FnOnce(&V) -> Option<&T>,
     {
@@ -113,7 +113,7 @@ impl<'a, K: Eq + Hash, V> RefMut<'a, K, V> {
         )
     }
 
-    pub fn map<F, T>(self, f: F) -> MappedRefMut<'a, K, T>
+    pub fn map<F, T: ?Sized>(self, f: F) -> MappedRefMut<'a, K, T>
     where
         F: FnOnce(&mut V) -> &mut T,
     {
@@ -124,7 +124,7 @@ impl<'a, K: Eq + Hash, V> RefMut<'a, K, V> {
         }
     }
 
-    pub fn try_map<F, T>(self, f: F) -> Result<MappedRefMut<'a, K, T>, Self>
+    pub fn try_map<F, T: ?Sized>(self, f: F) -> Result<MappedRefMut<'a, K, T>, Self>
     where
         F: FnOnce(&mut V) -> Option<&mut T>,
     {
@@ -165,13 +165,13 @@ impl<'a, K: Eq + Hash, V> DerefMut for RefMut<'a, K, V> {
     }
 }
 
-pub struct MappedRef<'a, K, T> {
+pub struct MappedRef<'a, K, T: ?Sized> {
     _guard: RwLockReadGuardDetached<'a>,
     k: &'a K,
     v: &'a T,
 }
 
-impl<'a, K: Eq + Hash, T> MappedRef<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized> MappedRef<'a, K, T> {
     pub fn key(&self) -> &K {
         self.pair().0
     }
@@ -195,7 +195,7 @@ impl<'a, K: Eq + Hash, T> MappedRef<'a, K, T> {
         }
     }
 
-    pub fn try_map<F, T2>(self, f: F) -> Result<MappedRef<'a, K, T2>, Self>
+    pub fn try_map<F, T2: ?Sized>(self, f: F) -> Result<MappedRef<'a, K, T2>, Self>
     where
         F: FnOnce(&T) -> Option<&T2>,
     {
@@ -212,7 +212,7 @@ impl<'a, K: Eq + Hash, T> MappedRef<'a, K, T> {
     }
 }
 
-impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRef<'a, K, T> {
+impl<'a, K: Eq + Hash + Debug, T: Debug + ?Sized> Debug for MappedRef<'a, K, T> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("MappedRef")
             .field("k", &self.k)
@@ -221,33 +221,35 @@ impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRef<'a, K, T> {
     }
 }
 
-impl<'a, K: Eq + Hash, T> Deref for MappedRef<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized> Deref for MappedRef<'a, K, T> {
     type Target = T;
 
     fn deref(&self) -> &T {
         self.value()
     }
 }
 
-impl<'a, K: Eq + Hash, T: std::fmt::Display> std::fmt::Display for MappedRef<'a, K, T> {
+impl<'a, K: Eq + Hash, T: std::fmt::Display + ?Sized> std::fmt::Display for MappedRef<'a, K, T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         std::fmt::Display::fmt(self.value(), f)
     }
 }
 
-impl<'a, K: Eq + Hash, T: AsRef<TDeref>, TDeref: ?Sized> AsRef<TDeref> for MappedRef<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized + AsRef<TDeref>, TDeref: ?Sized> AsRef<TDeref>
+    for MappedRef<'a, K, T>
+{
     fn as_ref(&self) -> &TDeref {
         self.value().as_ref()
     }
 }
 
-pub struct MappedRefMut<'a, K, T> {
+pub struct MappedRefMut<'a, K, T: ?Sized> {
     _guard: RwLockWriteGuardDetached<'a>,
     k: &'a K,
     v: &'a mut T,
 }
 
-impl<'a, K: Eq + Hash, T> MappedRefMut<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized> MappedRefMut<'a, K, T> {
     pub fn key(&self) -> &K {
         self.pair().0
     }
@@ -268,7 +270,7 @@ impl<'a, K: Eq + Hash, T> MappedRefMut<'a, K, T> {
         (self.k, self.v)
     }
 
-    pub fn map<F, T2>(self, f: F) -> MappedRefMut<'a, K, T2>
+    pub fn map<F, T2: ?Sized>(self, f: F) -> MappedRefMut<'a, K, T2>
     where
         F: FnOnce(&mut T) -> &mut T2,
     {
@@ -279,7 +281,7 @@ impl<'a, K: Eq + Hash, T> MappedRefMut<'a, K, T> {
         }
     }
 
-    pub fn try_map<F, T2>(self, f: F) -> Result<MappedRefMut<'a, K, T2>, Self>
+    pub fn try_map<F, T2: ?Sized>(self, f: F) -> Result<MappedRefMut<'a, K, T2>, Self>
     where
         F: FnOnce(&mut T) -> Option<&mut T2>,
     {
@@ -297,7 +299,7 @@ impl<'a, K: Eq + Hash, T> MappedRefMut<'a, K, T> {
     }
 }
 
-impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRefMut<'a, K, T> {
+impl<'a, K: Eq + Hash + Debug, T: Debug + ?Sized> Debug for MappedRefMut<'a, K, T> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("MappedRefMut")
             .field("k", &self.k)
@@ -306,16 +308,80 @@ impl<'a, K: Eq + Hash + Debug, T: Debug> Debug for MappedRefMut<'a, K, T> {
     }
 }
 
-impl<'a, K: Eq + Hash, T> Deref for MappedRefMut<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized> Deref for MappedRefMut<'a, K, T> {
     type Target = T;
 
     fn deref(&self) -> &T {
         self.value()
     }
 }
 
-impl<'a, K: Eq + Hash, T> DerefMut for MappedRefMut<'a, K, T> {
+impl<'a, K: Eq + Hash, T: ?Sized> DerefMut for MappedRefMut<'a, K, T> {
     fn deref_mut(&mut self) -> &mut T {
         self.value_mut()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::DashMap;
+
+    #[test]
+    fn downgrade() {
+        let data = DashMap::new();
+        data.insert("test", "test");
+        if let Some(mut w_ref) = data.get_mut("test") {
+            *w_ref.value_mut() = "test2";
+            let r_ref = w_ref.downgrade();
+            assert_eq!(*r_ref.value(), "test2");
+        };
+    }
+
+    #[test]
+    fn mapped_mut() {
+        let data = DashMap::new();
+        data.insert("test", *b"test");
+        if let Some(b_ref) = data.get_mut("test") {
+            let mut s_ref = b_ref.try_map(|b| std::str::from_utf8_mut(b).ok()).unwrap();
+            s_ref.value_mut().make_ascii_uppercase();
+        }
+
+        assert_eq!(data.get("test").unwrap().value(), b"TEST");
+    }
+
+    #[test]
+    fn mapped_mut_again() {
+        let data = DashMap::new();
+        data.insert("test", *b"hello world");
+        if let Some(b_ref) = data.get_mut("test") {
+            let s_ref = b_ref.try_map(|b| std::str::from_utf8_mut(b).ok()).unwrap();
+            let mut hello_ref = s_ref.try_map(|s| s.get_mut(..5)).unwrap();
+            hello_ref.value_mut().make_ascii_uppercase();
+        }
+
+        assert_eq!(data.get("test").unwrap().value(), b"HELLO world");
+    }
+
+    #[test]
+    fn mapped_ref() {
+        let data = DashMap::new();
+        data.insert("test", *b"test");
+        if let Some(b_ref) = data.get("test") {
+            let s_ref = b_ref.try_map(|b| std::str::from_utf8(b).ok()).unwrap();
+
+            assert_eq!(s_ref.value(), "test");
+        };
+    }
+
+    #[test]
+    fn mapped_ref_again() {
+        let data = DashMap::new();
+        data.insert("test", *b"hello world");
+        if let Some(b_ref) = data.get("test") {
+            let s_ref = b_ref.try_map(|b| std::str::from_utf8(b).ok()).unwrap();
+            let hello_ref = s_ref.try_map(|s| s.get(..5)).unwrap();
+
+            assert_eq!(hello_ref.value(), "hello");
+        };
+    }
+}

src/util.rs

@@ -113,11 +113,15 @@ impl<'a, R: RawRwLockDowngrade> RwLockWriteGuardDetached<'a, R> {
     ///
     /// The associated data must not mut mutated after downgrading
     pub(crate) unsafe fn downgrade(self) -> RwLockReadGuardDetached<'a, R> {
+        // Do not drop the write guard - otherwise we will trigger a downgrade + unlock_exclusive,
+        // which is incorrect
+        let this = ManuallyDrop::new(self);
+
         // Safety: An RwLockWriteGuardDetached always holds an exclusive lock.
-        unsafe { self.lock.downgrade() }
+        unsafe { this.lock.downgrade() }
         RwLockReadGuardDetached {
-            lock: self.lock,
-            _marker: self._marker,
+            lock: this.lock,
+            _marker: this._marker,
         }
     }
 }