feature gate deprecated APIs for GILPool (#4181)

src/gil.rs

@@ -1,5 +1,6 @@
 //! Interaction with Python's global interpreter lock
 
+#[cfg(feature = "gil-refs")]
 use crate::impl_::not_send::{NotSend, NOT_SEND};
 #[cfg(pyo3_disable_reference_pool)]
 use crate::impl_::panic::PanicTrap;
@@ -127,19 +128,16 @@ where
 
     ffi::Py_InitializeEx(0);
 
-    // Safety: the GIL is already held because of the Py_IntializeEx call.
+    let result = {
-    #[allow(deprecated)] // TODO: remove this with the GIL Refs feature in 0.22
+        let guard = GILGuard::assume();
-    let pool = GILPool::new();
+        let py = guard.python();
+        // Import the threading module - this ensures that it will associate this thread as the "main"
+        // thread, which is important to avoid an `AssertionError` at finalization.
+        py.import_bound("threading").unwrap();
 
-    // Import the threading module - this ensures that it will associate this thread as the "main"
+        // Execute the closure.
-    // thread, which is important to avoid an `AssertionError` at finalization.
+        f(py)
-    pool.python().import_bound("threading").unwrap();
+    };
-
-    // Execute the closure.
-    let result = f(pool.python());
-
-    // Drop the pool before finalizing.
-    drop(pool);
 
     // Finalize the Python interpreter.
     ffi::Py_Finalize();
@@ -154,7 +152,8 @@ pub(crate) enum GILGuard {
     /// Indicates that we actually acquired the GIL when this GILGuard was acquired
     Ensured {
         gstate: ffi::PyGILState_STATE,
-        #[allow(deprecated)] // TODO: remove this with the gil-refs feature in 0.22
+        #[cfg(feature = "gil-refs")]
+        #[allow(deprecated)]
         pool: mem::ManuallyDrop<GILPool>,
     },
 }
@@ -221,12 +220,23 @@ impl GILGuard {
         }
 
         let gstate = unsafe { ffi::PyGILState_Ensure() }; // acquire GIL
+        #[cfg(feature = "gil-refs")]
         #[allow(deprecated)]
-        let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };
+        {
+            let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };
+            increment_gil_count();
+            GILGuard::Ensured { gstate, pool }
+        }
 
-        increment_gil_count();
+        #[cfg(not(feature = "gil-refs"))]
+        {
+            increment_gil_count();
+            // Update counts of PyObjects / Py that have been cloned or dropped since last acquisition
+            #[cfg(not(pyo3_disable_reference_pool))]
+            POOL.update_counts(unsafe { Python::assume_gil_acquired() });
 
-        GILGuard::Ensured { gstate, pool }
+            GILGuard::Ensured { gstate }
+        }
     }
     /// Acquires the `GILGuard` while assuming that the GIL is already held.
     pub(crate) unsafe fn assume() -> Self {
@@ -246,12 +256,17 @@ impl Drop for GILGuard {
     fn drop(&mut self) {
         match self {
             GILGuard::Assumed => {}
+            #[cfg(feature = "gil-refs")]
             GILGuard::Ensured { gstate, pool } => unsafe {
                 // Drop the objects in the pool before attempting to release the thread state
                 mem::ManuallyDrop::drop(pool);
 
                 ffi::PyGILState_Release(*gstate);
             },
+            #[cfg(not(feature = "gil-refs"))]
+            GILGuard::Ensured { gstate } => unsafe {
+                ffi::PyGILState_Release(*gstate);
+            },
         }
         decrement_gil_count();
     }
@@ -368,12 +383,10 @@ impl Drop for LockGIL {
 
 ///
 /// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
-#[cfg_attr(
+#[cfg(feature = "gil-refs")]
-    not(feature = "gil-refs"),
+#[deprecated(
-    deprecated(
+    since = "0.21.0",
-        since = "0.21.0",
+    note = "`GILPool` has no function if PyO3's deprecated GIL Refs API is not used"
-        note = "`GILPool` has no function if PyO3's deprecated GIL Refs API is not used"
-    )
 )]
 pub struct GILPool {
     /// Initial length of owned objects and anys.
@@ -382,6 +395,7 @@ pub struct GILPool {
     _not_send: NotSend,
 }
 
+#[cfg(feature = "gil-refs")]
 #[allow(deprecated)]
 impl GILPool {
     /// Creates a new [`GILPool`]. This function should only ever be called with the GIL held.
@@ -419,6 +433,7 @@ impl GILPool {
     }
 }
 
+#[cfg(feature = "gil-refs")]
 #[allow(deprecated)]
 impl Drop for GILPool {
     fn drop(&mut self) {
@@ -534,19 +549,15 @@ fn decrement_gil_count() {
 
 #[cfg(test)]
 mod tests {
-    #[allow(deprecated)]
-    use super::GILPool;
     #[cfg(not(pyo3_disable_reference_pool))]
-    use super::POOL;
+    use super::{gil_is_acquired, POOL};
-    use super::{gil_is_acquired, GIL_COUNT};
-    #[cfg(not(pyo3_disable_reference_pool))]
-    use crate::ffi;
-    use crate::types::any::PyAnyMethods;
-    use crate::{PyObject, Python};
     #[cfg(feature = "gil-refs")]
-    use {super::OWNED_OBJECTS, crate::gil};
+    #[allow(deprecated)]
-
+    use super::{GILPool, GIL_COUNT, OWNED_OBJECTS};
-    #[cfg(not(pyo3_disable_reference_pool))]
+    use crate::types::any::PyAnyMethods;
+    #[cfg(feature = "gil-refs")]
+    use crate::{ffi, gil};
+    use crate::{PyObject, Python};
     use std::ptr::NonNull;
 
     fn get_object(py: Python<'_>) -> PyObject {
@@ -691,6 +702,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(feature = "gil-refs")]
     #[allow(deprecated)]
     fn test_gil_counts() {
         // Check with_gil and GILPool both increase counts correctly
@@ -782,6 +794,7 @@ mod tests {
 
     #[test]
     #[cfg(not(pyo3_disable_reference_pool))]
+    #[cfg(feature = "gil-refs")]
     fn test_update_counts_does_not_deadlock() {
         // update_counts can run arbitrary Python code during Py_DECREF.
         // if the locking is implemented incorrectly, it will deadlock.

src/impl_/not_send.rs

@@ -6,4 +6,5 @@ use crate::Python;
 /// Workaround for lack of !Send on stable (<https://github.com/rust-lang/rust/issues/68318>).
 pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);
 
+#[cfg(feature = "gil-refs")]
 pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);

src/lib.rs

@@ -323,6 +323,7 @@ pub use crate::conversion::{FromPyPointer, PyTryFrom, PyTryInto};
 #[cfg(feature = "gil-refs")]
 pub use crate::err::PyDowncastError;
 pub use crate::err::{DowncastError, DowncastIntoError, PyErr, PyErrArguments, PyResult, ToPyErr};
+#[cfg(feature = "gil-refs")]
 #[allow(deprecated)]
 pub use crate::gil::GILPool;
 #[cfg(not(any(PyPy, GraalPy)))]

src/marker.rs

@@ -126,12 +126,10 @@ use crate::types::{
     PyAny, PyDict, PyEllipsis, PyModule, PyNone, PyNotImplemented, PyString, PyType,
 };
 use crate::version::PythonVersionInfo;
-#[cfg(feature = "gil-refs")]
-use crate::PyNativeType;
 use crate::{ffi, Bound, IntoPy, Py, PyObject, PyTypeInfo};
 #[allow(deprecated)]
 #[cfg(feature = "gil-refs")]
-use crate::{gil::GILPool, FromPyPointer};
+use crate::{gil::GILPool, FromPyPointer, PyNativeType};
 use std::ffi::{CStr, CString};
 use std::marker::PhantomData;
 use std::os::raw::c_int;