fix memory leak

Signed-off-by: MrCroxx <[email protected]>
foyer-rs · Jun 30, 2023 · 6776008 · 6776008
1 parent 04b83f5
commit 6776008
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 26 deletions.
diff --git a/foyer-intrusive/src/collections/dlist.rs b/foyer-intrusive/src/collections/dlist.rs
@@ -54,6 +54,20 @@ where
     adapter: A,
 }
 
+impl<A> Drop for DList<A>
+where
+    A: Adapter<Link = DListLink>,
+{
+    fn drop(&mut self) {
+        let mut iter = self.iter_mut();
+        iter.front();
+        while iter.is_valid() {
+            iter.remove();
+        }
+        assert!(self.is_empty());
+    }
+}
+
 impl<A> DList<A>
 where
     A: Adapter<Link = DListLink>,
@@ -199,12 +213,12 @@ where
     }
 
     /// Move to head.
-    pub fn head(&mut self) {
+    pub fn front(&mut self) {
         self.link = self.dlist.head;
     }
 
     /// Move to head.
-    pub fn tail(&mut self) {
+    pub fn back(&mut self) {
         self.link = self.dlist.tail;
     }
 }
@@ -430,9 +444,11 @@ where
 
 #[cfg(test)]
 mod tests {
+    use std::sync::Arc;
+
     use itertools::Itertools;
 
-    use crate::core::pointer::DefaultPointerOps;
+    use crate::{core::pointer::DefaultPointerOps, intrusive_adapter};
 
     use super::*;
 
@@ -482,6 +498,8 @@ mod tests {
         }
     }
 
+    intrusive_adapter! { DListArcAdapter = Arc<DListItem>: DListItem { link: DListLink } }
+
     #[test]
     fn test_dlist_simple() {
         let mut l = DList::<DListAdapter>::new();
@@ -512,4 +530,21 @@ mod tests {
         assert!(l.pop_front().is_none());
         assert_eq!(l.len(), 0);
     }
+
+    #[test]
+    fn test_arc_drop() {
+        let mut l = DList::<DListArcAdapter>::new();
+
+        let items = (0..10).map(|i| Arc::new(DListItem::new(i))).collect_vec();
+        for item in items.iter() {
+            l.push_back(item.clone());
+        }
+        for item in items.iter() {
+            assert_eq!(Arc::strong_count(item), 2);
+        }
+        drop(l);
+        for item in items.iter() {
+            assert_eq!(Arc::strong_count(item), 1);
+        }
+    }
 }
diff --git a/foyer-intrusive/src/core/adapter.rs b/foyer-intrusive/src/core/adapter.rs
@@ -89,23 +89,22 @@ pub unsafe trait KeyAdapter: Adapter {
 ///
 /// ```
 /// use foyer_intrusive::{intrusive_adapter, key_adapter};
-/// use foyer_intrusive::core::adapter::{Adapter, KeyAdapter};
+/// use foyer_intrusive::core::adapter::{Adapter, KeyAdapter, Link};
 /// use foyer_intrusive::core::pointer::PointerOps;
 /// use foyer_intrusive::eviction::EvictionPolicy;
 /// use std::sync::Arc;
 ///
-/// pub struct Item<A, E>
+/// #[derive(Debug)]
+/// pub struct Item<L>
 /// where
-///     E: EvictionPolicy<A>,
-///     A: KeyAdapter<Link = E::Link>,
-///     <<A as Adapter>::PointerOps as PointerOps>::Pointer: Clone,
+///     L: Link
 /// {
-///     link: E::Link,
+///     link: L,
 ///     key: u64,
 /// }
 ///
-/// intrusive_adapter! { ItemAdapter<E> = Arc<Item<ItemAdapter<E>, E>>: Item<ItemAdapter<E>, E> { link: E::Link} where E:EvictionPolicy<ItemAdapter<E>> }
-/// key_adapter! { ItemAdapter<E> = Item<ItemAdapter<E>, E> { key: u64 } where E: EvictionPolicy<ItemAdapter<E>> }
+/// intrusive_adapter! { ItemAdapter<L> = Arc<Item<L>>: Item<L> { link: L} where L: Link }
+/// key_adapter! { ItemAdapter<L> = Item<L> { key: u64 } where L: Link }
 /// ```
 #[macro_export]
 macro_rules! intrusive_adapter {
@@ -186,23 +185,22 @@ macro_rules! intrusive_adapter {
 ///
 /// ```
 /// use foyer_intrusive::{intrusive_adapter, key_adapter};
-/// use foyer_intrusive::core::adapter::{Adapter, KeyAdapter};
+/// use foyer_intrusive::core::adapter::{Adapter, KeyAdapter, Link};
 /// use foyer_intrusive::core::pointer::PointerOps;
 /// use foyer_intrusive::eviction::EvictionPolicy;
 /// use std::sync::Arc;
 ///
-/// pub struct Item<A, E>
+/// #[derive(Debug)]
+/// pub struct Item<L>
 /// where
-///     E: EvictionPolicy<A>,
-///     A: KeyAdapter<Link = E::Link>,
-///     <<A as Adapter>::PointerOps as PointerOps>::Pointer: Clone,
+///     L: Link
 /// {
-///     link: E::Link,
+///     link: L,
 ///     key: u64,
 /// }
 ///
-/// intrusive_adapter! { ItemAdapter<E> = Arc<Item<ItemAdapter<E>, E>>: Item<ItemAdapter<E>, E> { link: E::Link} where E:EvictionPolicy<ItemAdapter<E>> }
-/// key_adapter! { ItemAdapter<E> = Item<ItemAdapter<E>, E> { key: u64 } where E: EvictionPolicy<ItemAdapter<E>> }
+/// intrusive_adapter! { ItemAdapter<L> = Arc<Item<L>>: Item<L> { link: L} where L: Link }
+/// key_adapter! { ItemAdapter<L> = Item<L> { key: u64 } where L: Link }
 /// ```
 #[macro_export]
 macro_rules! key_adapter {

diff --git a/foyer-intrusive/src/eviction/lfu.rs b/foyer-intrusive/src/eviction/lfu.rs
@@ -154,10 +154,25 @@ where
     adapter: A,
 }
 
-impl<A> Lfu<A>
+impl<A> Drop for Lfu<A>
 where
     A: KeyAdapter<Link = LfuLink>,
+    <<A as Adapter>::PointerOps as PointerOps>::Pointer: Clone,
+{
+    fn drop(&mut self) {
+        let mut to_remove = vec![];
+        for ptr in self.iter() {
+            to_remove.push(ptr.clone());
+        }
+        for ptr in to_remove {
+            self.remove(&ptr);
+        }
+    }
+}
 
+impl<A> Lfu<A>
+where
+    A: KeyAdapter<Link = LfuLink>,
     <<A as Adapter>::PointerOps as PointerOps>::Pointer: Clone,
 {
     pub fn new(config: LfuConfig) -> Self {
@@ -241,8 +256,8 @@ where
         let mut iter_main = self.lru_main.iter();
         let mut iter_tiny = self.lru_tiny.iter();
 
-        iter_main.tail();
-        iter_tiny.tail();
+        iter_main.back();
+        iter_tiny.back();
 
         LfuIter {
             lfu: self,

diff --git a/foyer-intrusive/src/eviction/lru.rs b/foyer-intrusive/src/eviction/lru.rs
@@ -85,6 +85,22 @@ where
     adapter: A,
 }
 
+impl<A> Drop for Lru<A>
+where
+    A: KeyAdapter<Link = LruLink>,
+    <<A as Adapter>::PointerOps as PointerOps>::Pointer: Clone,
+{
+    fn drop(&mut self) {
+        let mut to_remove = vec![];
+        for ptr in self.iter() {
+            to_remove.push(ptr.clone());
+        }
+        for ptr in to_remove {
+            self.remove(&ptr);
+        }
+    }
+}
+
 impl<A> Lru<A>
 where
     A: KeyAdapter<Link = LruLink>,
@@ -162,7 +178,7 @@ where
 
     fn iter(&self) -> LruIter<'_, A> {
         let mut iter = self.lru.iter();
-        iter.tail();
+        iter.back();
         LruIter {
             iter,
             lru: self,

diff --git a/foyer-storage/src/region_manager.rs b/foyer-storage/src/region_manager.rs
@@ -38,9 +38,6 @@ where
     id: RegionId,
 }
 
-// intrusive_adapter! { pub RegionEpItemAdapter<E, L> = Arc<RegionEpItem<L>>: RegionEpItem<L> { link: L } where L: Link, E: EvictionPolicy<RegionEpItemAdapter<E, L>, Link = L> }
-// key_adapter! { RegionEpItemAdapter<E, L> = RegionEpItem<L> { id: RegionId } where L: Link, E: EvictionPolicy<RegionEpItemAdapter<E, L>, Link = L> }
-
 intrusive_adapter! { pub RegionEpItemAdapter<L> = Arc<RegionEpItem<L>>: RegionEpItem<L> { link: L } where L: Link }
 key_adapter! { RegionEpItemAdapter<L> = RegionEpItem<L> { id: RegionId } where L: Link }