Remove destroy from FixedPool (#7602)

turns out we don't need it
pyca · Sep 11, 2022 · 30114c6 · 30114c6
1 parent d480268
commit 30114c6
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 35 deletions.
diff --git a/src/cryptography/hazmat/bindings/_rust/__init__.pyi b/src/cryptography/hazmat/bindings/_rust/__init__.pyi
@@ -20,7 +20,6 @@ class FixedPool(typing.Generic[T]):
     def __init__(
         self,
         create: typing.Callable[[], T],
-        destroy: typing.Callable[[T], None],
     ) -> None: ...
     def acquire(self) -> PoolAcquisition[T]: ...
 

diff --git a/src/rust/src/pool.rs b/src/rust/src/pool.rs
@@ -10,7 +10,6 @@ use std::cell::Cell;
 #[pyo3::prelude::pyclass]
 pub(crate) struct FixedPool {
     create_fn: pyo3::PyObject,
-    destroy_fn: pyo3::PyObject,
 
     value: Cell<Option<pyo3::PyObject>>,
 }
@@ -26,16 +25,11 @@ struct PoolAcquisition {
 #[pyo3::pymethods]
 impl FixedPool {
     #[new]
-    fn new(
-        py: pyo3::Python<'_>,
-        create: pyo3::PyObject,
-        destroy: pyo3::PyObject,
-    ) -> pyo3::PyResult<Self> {
+    fn new(py: pyo3::Python<'_>, create: pyo3::PyObject) -> pyo3::PyResult<Self> {
         let value = create.call0(py)?;
 
         Ok(FixedPool {
             create_fn: create,
-            destroy_fn: destroy,
 
             value: Cell::new(Some(value)),
         })
@@ -60,18 +54,6 @@ impl FixedPool {
     }
 }
 
-impl Drop for FixedPool {
-    fn drop(&mut self) {
-        if let Some(value) = self.value.replace(None) {
-            let gil = pyo3::Python::acquire_gil();
-            let py = gil.python();
-            self.destroy_fn
-                .call1(py, (value,))
-                .expect("FixedPool destroy function failed in destructor");
-        }
-    }
-}
-
 #[pyo3::pymethods]
 impl PoolAcquisition {
     fn __enter__(&self, py: pyo3::Python<'_>) -> pyo3::PyObject {
@@ -86,9 +68,7 @@ impl PoolAcquisition {
         _exc_tb: &pyo3::PyAny,
     ) -> pyo3::PyResult<()> {
         let pool = self.pool.as_ref(py).borrow();
-        if self.fresh {
-            pool.destroy_fn.call1(py, (self.value.clone_ref(py),))?;
-        } else {
+        if !self.fresh {
             pool.value.replace(Some(self.value.clone_ref(py)));
         }
         Ok(())

diff --git a/tests/test_rust_utils.py b/tests/test_rust_utils.py
@@ -19,10 +19,7 @@ def create():
             events.append(("create", c))
             return c
 
-        def destroy(c):
-            events.append(("destroy", c))
-
-        pool = FixedPool(create, destroy)
+        pool = FixedPool(create)
         assert events == [("create", 1)]
         with pool.acquire() as c:
             assert c == 1
@@ -32,9 +29,9 @@ def destroy(c):
                 assert c == 2
                 assert events == [("create", 1), ("create", 2)]
 
-            assert events == [("create", 1), ("create", 2), ("destroy", 2)]
+            assert events == [("create", 1), ("create", 2)]
 
-        assert events == [("create", 1), ("create", 2), ("destroy", 2)]
+        assert events == [("create", 1), ("create", 2)]
 
         del pool
         gc.collect()
@@ -44,18 +41,13 @@ def destroy(c):
         assert events == [
             ("create", 1),
             ("create", 2),
-            ("destroy", 2),
-            ("destroy", 1),
         ]
 
     def test_thread_stress(self):
         def create():
             return None
 
-        def destroy(c):
-            pass
-
-        pool = FixedPool(create, destroy)
+        pool = FixedPool(create)
 
         def thread_fn():
             with pool.acquire():