Merge pull request #1848 from input-output-hk/jpraynaud/fix-resource-pool-reset

fix: Resource not Reset when given back to Pool

Author: jpraynaud

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-common"
-version = "0.4.33"
+version = "0.4.34"
 description = "Common types, interfaces, and utilities for Mithril nodes."
 authors = { workspace = true }
 edition = { workspace = true }

mithril-common/Cargo.toml

@@ -76,6 +76,8 @@ impl<T: Reset + Send + Sync> ResourcePool<T> {
     /// A resource is given back to the pool only if the discriminant matches
     /// and if the pool is not already full
     pub fn give_back_resource(&self, resource: T, discriminant: u64) -> StdResult<()> {
+        let mut resource = resource;
+        resource.reset()?;
         if self.count()? == self.size {
             // Pool is full
             return Ok(());
@@ -103,12 +105,9 @@ impl<T: Reset + Send + Sync> ResourcePool<T> {
         resource_pool_item: ResourcePoolItem<'_, T>,
     ) -> StdResult<()> {
         let mut resource_pool_item = resource_pool_item;
-        resource_pool_item.take().map(|resource_item| {
-            let mut resource_item = resource_item;
-            resource_item.reset()?;
-
-            self.give_back_resource(resource_item, self.discriminant()?)
-        });
+        resource_pool_item
+            .take()
+            .map(|resource_item| self.give_back_resource(resource_item, self.discriminant()?));
 
         Ok(())
     }
@@ -228,6 +227,20 @@ mod tests {
 
     use super::*;
 
+    /// Resource for testing reset
+    #[derive(Default)]
+    struct TestResetResource {
+        reset: bool,
+    }
+
+    impl Reset for TestResetResource {
+        fn reset(&mut self) -> StdResult<()> {
+            self.reset = true;
+
+            Ok(())
+        }
+    }
+
     #[test]
     fn test_resource_pool_acquire_returns_resource_when_available() {
         let pool_size = 10;
@@ -369,4 +382,30 @@ mod tests {
 
         assert_eq!(pool.count().unwrap(), pool_size - 1);
     }
+
+    #[tokio::test]
+    async fn test_resource_pool_is_reset_when_given_back() {
+        let pool = ResourcePool::<TestResetResource>::new(1, vec![TestResetResource::default()]);
+
+        let mut resource_item = pool.acquire_resource(Duration::from_millis(10)).unwrap();
+        let resource = resource_item.take().unwrap();
+        pool.give_back_resource(resource, pool.discriminant().unwrap())
+            .unwrap();
+
+        // Acquire the resource again and make sure it has been reseted
+        let resource_item = pool.acquire_resource(Duration::from_millis(10)).unwrap();
+        assert!(resource_item.reset);
+    }
+
+    #[tokio::test]
+    async fn test_resource_pool_item_is_reset_when_given_back() {
+        let pool = ResourcePool::<TestResetResource>::new(1, vec![TestResetResource::default()]);
+
+        let resource_item = pool.acquire_resource(Duration::from_millis(10)).unwrap();
+        pool.give_back_resource_pool_item(resource_item).unwrap();
+
+        // Acquire the resource again and make sure it has been reseted
+        let resource_item = pool.acquire_resource(Duration::from_millis(10)).unwrap();
+        assert!(resource_item.reset);
+    }
 }