fix syncvec/syncmap drop's bug

co-rs · Feb 20, 2022 · 61a04ab · 61a04ab
1 parent cba3cdf
commit 61a04ab
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 28 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -6,7 +6,7 @@ members = [
 
 [package]
 name = "mco"
-version = "0.1.40"
+version = "0.1.41"
 edition = "2018"
 authors = ["[email protected]", "Xudong Huang <[email protected]>"]
 license = "MIT/Apache-2.0"

diff --git a/src/std/lazy/mod.rs b/src/std/lazy/mod.rs
@@ -4,13 +4,13 @@
 //! might store arbitrary non-`Copy` types, can be assigned to at most once and provides direct access
 //! to the stored contents. The core API looks *roughly* like this (and there's much more inside, read on!):
 //!
-//! ```rust,ignore
+//!
 //! impl<T> OnceCell<T> {
 //!     const fn new() -> OnceCell<T> { ... }
 //!     fn set(&self, value: T) -> Result<(), T> { ... }
 //!     fn get(&self) -> Option<&T> { ... }
 //! }
-//! ```
+//!
 //!
 //! Note that, like with [`RefCell`] and [`Mutex`], the `set` method requires only a shared reference.
 //! Because of the single assignment restriction `get` can return a `&T` instead of `Ref<T>`

diff --git a/src/std/map/btree_map.rs b/src/std/map/btree_map.rs
@@ -34,19 +34,39 @@ pub type SyncBtreeMap<K, V> = SyncMapImpl<K, V>;
 /// contention compared to a Go map paired with a separate Mutex or RWMutex.
 ///
 /// The zero Map is empty and ready for use. A Map must not be copied after first use.
-pub struct SyncMapImpl<K, V> {
+pub struct SyncMapImpl<K: Eq + Hash + Clone + Ord, V> {
     read: UnsafeCell<Map<K, V>>,
     dirty: Mutex<HashMap<K, V>>,
 }
 
+impl<K: Eq + Hash + Clone + Ord, V> Drop for SyncMapImpl<K, V> {
+    fn drop(&mut self) {
+        unsafe {
+            let mut keys = Vec::with_capacity(self.len());
+            for (k, _) in (&mut *self.read.get()) {
+                keys.insert(0, k);
+            }
+            for x in keys {
+                let v = (&mut *self.read.get()).remove(x);
+                match v {
+                    None => {}
+                    Some(v) => {
+                        std::mem::forget(v);
+                    }
+                }
+            }
+        }
+    }
+}
+
 /// this is safety, dirty mutex ensure
-unsafe impl<K, V> Send for SyncMapImpl<K, V> {}
+unsafe impl<K: Eq + Hash + Clone + Ord, V> Send for SyncMapImpl<K, V> {}
 
 /// this is safety, dirty mutex ensure
-unsafe impl<K, V> Sync for SyncMapImpl<K, V> {}
+unsafe impl<K: Eq + Hash + Clone + Ord, V> Sync for SyncMapImpl<K, V> {}
 
 //TODO maybe K will use transmute_copy replace Clone?
-impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
+impl<K: Eq + Hash + Clone + Ord, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
     pub fn new_arc() -> Arc<Self> {
         Arc::new(Self::new())
     }
@@ -86,15 +106,15 @@ impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
         }
     }
 
-    pub fn remove(&self, k: &K) -> Option<V> where K: Clone+ std::cmp::Ord {
+    pub fn remove(&self, k: &K) -> Option<V> where K: Clone + std::cmp::Ord {
         match self.dirty.lock() {
             Ok(mut m) => {
                 let op = m.remove(k);
                 match op {
                     Some(v) => {
                         unsafe {
-                            let r=(&mut *self.read.get()).remove(k);
-                            match r{
+                            let r = (&mut *self.read.get()).remove(k);
+                            match r {
                                 None => {}
                                 Some(r) => {
                                     std::mem::forget(r);
@@ -131,8 +151,10 @@ impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
             Ok(mut m) => {
                 m.clear();
                 unsafe {
-                    for x in (&*self.read.get()).keys() {
-                        match (&mut *self.read.get()).remove(x){
+                    let k = (&mut *self.read.get()).keys().clone();
+                    for x in k {
+                        let v = (&mut *self.read.get()).remove(x);
+                        match v {
                             None => {}
                             Some(v) => {
                                 std::mem::forget(v);
@@ -224,7 +246,7 @@ impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
 
     pub fn iter(&self) -> MapIter<'_, K, V> {
         unsafe {
-           (&*self.read.get()).iter()
+            (&*self.read.get()).iter()
         }
     }
 
@@ -350,7 +372,7 @@ impl<'a, K, V> Iterator for IterMut<'a, K, V> {
     }
 }
 
-impl<'a, K, V> IntoIterator for &'a SyncMapImpl<K, V> where K: Eq + Hash + Clone {
+impl<'a, K: Eq + Hash + Clone + Ord, V> IntoIterator for &'a SyncMapImpl<K, V> {
     type Item = (&'a K, &'a V);
     type IntoIter = MapIter<'a, K, V>;
 
@@ -360,7 +382,7 @@ impl<'a, K, V> IntoIterator for &'a SyncMapImpl<K, V> where K: Eq + Hash + Clone
 }
 
 
-impl<'a, K, V> IntoIterator for &'a mut SyncMapImpl<K, V> where K: Eq + Hash + Clone {
+impl<'a, K: Eq + Hash + Clone + Ord, V> IntoIterator for &'a mut SyncMapImpl<K, V> {
     type Item = (&'a K, &'a mut V);
     type IntoIter = IterMut<'a, K, V>;
 
@@ -369,7 +391,7 @@ impl<'a, K, V> IntoIterator for &'a mut SyncMapImpl<K, V> where K: Eq + Hash + C
     }
 }
 
-impl<K, V> IntoIterator for SyncMapImpl<K, V> where
+impl<K: Eq + Hash + Clone + Ord, V> IntoIterator for SyncMapImpl<K, V> where
     K: Eq + Hash + Clone,
     K: 'static, V: 'static {
     type Item = (&'static K, &'static V);
@@ -381,13 +403,13 @@ impl<K, V> IntoIterator for SyncMapImpl<K, V> where
 }
 
 
-impl<K, V> From<Map<K, V>> for SyncMapImpl<K, V> {
-    fn from(arg: Map<K, V>) -> Self {
+impl<K: Eq + Hash + Clone + Ord, V> From<HashMap<K, V>> for SyncMapImpl<K, V> {
+    fn from(arg: HashMap<K, V>) -> Self {
         Self::from(arg)
     }
 }
 
-impl<K, V> serde::Serialize for SyncMapImpl<K, V> where K: Eq + Hash + Clone + Serialize, V: Serialize {
+impl<K: Eq + Hash + Clone + Ord, V> serde::Serialize for SyncMapImpl<K, V> where K: Eq + Hash + Clone + Serialize, V: Serialize {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: Serializer {
         let mut m = serializer.serialize_map(Some(self.len()))?;
         for (k, v) in self.iter() {
@@ -405,7 +427,7 @@ impl<'de, K, V> serde::Deserialize<'de> for SyncMapImpl<K, V> where K: Eq + Hash
     }
 }
 
-impl<K, V> Debug for SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone + Debug, V: Debug {
+impl<K: Eq + Hash + Clone + Ord, V> Debug for SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone + Debug, V: Debug {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         let mut m = f.debug_map();
         for (k, v) in self.iter() {

diff --git a/src/std/map/hash_map.rs b/src/std/map/hash_map.rs
@@ -34,16 +34,33 @@ pub type SyncHashMap<K, V> = SyncMapImpl<K, V>;
 /// contention compared to a Go map paired with a separate Mutex or RWMutex.
 ///
 /// The zero Map is empty and ready for use. A Map must not be copied after first use.
-pub struct SyncMapImpl<K, V> {
+pub struct SyncMapImpl<K:Eq+Hash+Clone, V> {
     read: UnsafeCell<Map<K, V>>,
     dirty: Mutex<Map<K, V>>,
 }
 
+impl <K:Eq+Hash+Clone, V>Drop for SyncMapImpl<K,V> {
+    fn drop(&mut self) {
+        unsafe {
+            let k=(&mut *self.read.get()).keys().clone();
+            for x in k {
+                let v=(&mut *self.read.get()).remove(x);
+                match v{
+                    None => {}
+                    Some(v) => {
+                        std::mem::forget(v);
+                    }
+                }
+            }
+        }
+    }
+}
+
 /// this is safety, dirty mutex ensure
-unsafe impl<K, V> Send for SyncMapImpl<K, V> {}
+unsafe impl<K:Eq+Hash+Clone, V> Send for SyncMapImpl<K, V> {}
 
 /// this is safety, dirty mutex ensure
-unsafe impl<K, V> Sync for SyncMapImpl<K, V> {}
+unsafe impl<K:Eq+Hash+Clone, V> Sync for SyncMapImpl<K, V> {}
 
 //TODO maybe K will use transmute_copy replace Clone?
 impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
@@ -134,8 +151,10 @@ impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
             Ok(mut m) => {
                 m.clear();
                 unsafe {
-                    for x in (&*self.read.get()).keys() {
-                        match (&mut *self.read.get()).remove(x){
+                    let k=(&mut *self.read.get()).keys().clone();
+                    for x in k {
+                        let v=(&mut *self.read.get()).remove(x);
+                        match v{
                             None => {}
                             Some(v) => {
                                 std::mem::forget(v);
@@ -144,7 +163,8 @@ impl<K, V> SyncMapImpl<K, V> where K: std::cmp::Eq + Hash + Clone {
                     }
                 }
             }
-            Err(_) => {}
+            Err(_) => {
+            }
         }
     }
 
@@ -394,7 +414,7 @@ impl<K, V> IntoIterator for SyncMapImpl<K, V> where
 }
 
 
-impl<K, V> From<Map<K, V>> for SyncMapImpl<K, V> {
+impl<K:Eq+Hash+Clone, V> From<Map<K, V>> for SyncMapImpl<K, V> {
     fn from(arg: Map<K, V>) -> Self {
         Self::from(arg)
     }

diff --git a/src/std/time/time.rs b/src/std/time/time.rs
@@ -571,8 +571,10 @@ mod test {
     fn test_smoke() {
         let mut now = Time::now();
         //is before?
+        sleep(Duration::from_secs(1));
         assert_eq!(true, now.before(&Time::now()));
         //is after?
+        sleep(Duration::from_secs(1));
         assert_eq!(true, Time::now().after(&now));
         //parse from str
         let parsed = Time::parse(RFC3339Nano, &now.to_string()).unwrap();

diff --git a/src/std/vec/mod.rs b/src/std/vec/mod.rs
@@ -18,6 +18,23 @@ pub struct SyncVecImpl<V> {
     dirty: Mutex<Vec<V>>,
 }
 
+impl<V> Drop for SyncVecImpl<V> {
+    fn drop(&mut self) {
+        unsafe {
+            loop {
+                match (&mut *self.read.get()).pop() {
+                    None => {
+                        break;
+                    }
+                    Some(v) => {
+                        std::mem::forget(v)
+                    }
+                }
+            }
+        }
+    }
+}
+
 /// this is safety, dirty mutex ensure
 unsafe impl<V> Send for SyncVecImpl<V> {}
 
@@ -87,7 +104,7 @@ impl<V> SyncVecImpl<V> {
                     Some(s) => {
                         unsafe {
                             let r = (&mut *self.read.get()).pop();
-                            match r{
+                            match r {
                                 None => {}
                                 Some(r) => {
                                     std::mem::forget(r);