deprecate GILPool (#3947)

* deprecate `GILPool`

* review: adamreichold

* fix deprecation warnings in tests

guide/src/memory.md

@@ -83,6 +83,13 @@ bound to the `GILPool`, not the for loop.  The `GILPool` isn't dropped until
 the end of the `with_gil()` closure, at which point the 10 copies of `hello`
 are finally released to the Python garbage collector.
 
+<div class="warning">
+⚠️ Warning: `GILPool` is no longer the preferred way to manage memory with PyO3 🛠️
+
+PyO3 0.21 has introduced a new API known as the Bound API, which doesn't have the same surprising results. Instead, each `Bound<T>` smart pointer releases the Python reference immediately on drop. See [the smart pointer types](./types.md#pyo3s-smart-pointers) for more details.
+</div>
+
+
 In general we don't want unbounded memory growth during loops!  One workaround
 is to acquire and release the GIL with each iteration of the loop.
 
@@ -114,6 +121,7 @@ this is unsafe.
 # fn main() -> PyResult<()> {
 Python::with_gil(|py| -> PyResult<()> {
     for _ in 0..10 {
+        #[allow(deprecated)]  // `new_pool` is not needed in code not using the GIL Refs API
         let pool = unsafe { py.new_pool() };
         let py = pool.python();
         #[allow(deprecated)] // py.eval() is part of the GIL Refs API
@@ -146,7 +154,11 @@ function call, releasing objects when the function returns. Most functions only
 a few objects, meaning this doesn't have a significant impact. Occasionally functions
 with long complex loops may need to use `Python::new_pool` as shown above.
 
-This behavior may change in future, see [issue #1056](https://github.com/PyO3/pyo3/issues/1056).
+<div class="warning">
+⚠️ Warning: `GILPool` is no longer the preferred way to manage memory with PyO3 🛠️
+
+PyO3 0.21 has introduced a new API known as the Bound API, which doesn't have the same surprising results. Instead, each `Bound<T>` smart pointer releases the Python reference immediately on drop. See [the smart pointer types](./types.md#pyo3s-smart-pointers) for more details.
+</div>
 
 ## GIL-independent memory
 

guide/src/migration.md

@@ -280,6 +280,8 @@ The expectation is that in 0.22 `extract_bound` will have the default implementa
 
 As a final step of migration, deactivating the `gil-refs` feature will set up code for best performance and is intended to set up a forward-compatible API for PyO3 0.22.
 
+At this point code which needed to manage GIL Ref memory can safely remove uses of `GILPool` (which are constructed by calls to `Python::new_pool` and `Python::with_pool`). Deprecation warnings will highlight these cases.
+
 There is one notable API removed when this feature is disabled. `FromPyObject` trait implementations for types which borrow directly from the input data cannot be implemented by PyO3 without GIL Refs (while the migration is ongoing). These types are `&str`, `Cow<'_, str>`, `&[u8]`, `Cow<'_, u8>`.
 
 To ease pain during migration, these types instead implement a new temporary trait `FromPyObjectBound` which is the expected future form of `FromPyObject`. The new temporary trait ensures is that `obj.extract::<&str>()` continues to work (with the new constraint that the extracted value now depends on the input `obj` lifetime), as well for these types in `#[pyfunction]` arguments.

newsfragments/3947.changed.md

@@ -0,0 +1 @@
+Deprecate `GILPool`, `Python::with_pool`, and `Python::new_pool`.

src/gil.rs

@@ -139,6 +139,7 @@ where
     ffi::Py_InitializeEx(0);
 
     // Safety: the GIL is already held because of the Py_IntializeEx call.
+    #[allow(deprecated)] // TODO: remove this with the GIL Refs feature in 0.22
     let pool = GILPool::new();
 
     // Import the threading module - this ensures that it will associate this thread as the "main"
@@ -160,6 +161,7 @@ where
 /// RAII type that represents the Global Interpreter Lock acquisition.
 pub(crate) struct GILGuard {
     gstate: ffi::PyGILState_STATE,
+    #[allow(deprecated)] // TODO: remove this with the gil-refs feature in 0.22
     pool: mem::ManuallyDrop<GILPool>,
 }
 
@@ -222,6 +224,7 @@ impl GILGuard {
         }
 
         let gstate = unsafe { ffi::PyGILState_Ensure() }; // acquire GIL
+        #[allow(deprecated)]
         let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };
 
         Some(GILGuard { gstate, pool })
@@ -358,13 +361,21 @@ impl Drop for LockGIL {
 
 ///
 /// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
+#[cfg_attr(
+    not(feature = "gil-refs"),
+    deprecated(
+        since = "0.21.0",
+        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.
     /// `Option` is used since TSL can be broken when `new` is called from `atexit`.
     start: Option<usize>,
     _not_send: NotSend,
 }
 
+#[allow(deprecated)]
 impl GILPool {
     /// Creates a new [`GILPool`]. This function should only ever be called with the GIL held.
     ///
@@ -401,6 +412,7 @@ impl GILPool {
     }
 }
 
+#[allow(deprecated)]
 impl Drop for GILPool {
     fn drop(&mut self) {
         if let Some(start) = self.start {
@@ -506,21 +518,17 @@ fn decrement_gil_count() {
 
 #[cfg(test)]
 mod tests {
-    use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL};
+    #[allow(deprecated)]
+    use super::GILPool;
+    use super::{gil_is_acquired, GIL_COUNT, OWNED_OBJECTS, POOL};
     use crate::types::any::PyAnyMethods;
-    use crate::{ffi, gil, PyObject, Python, ToPyObject};
+    use crate::{ffi, gil, PyObject, Python};
     #[cfg(not(target_arch = "wasm32"))]
     use parking_lot::{const_mutex, Condvar, Mutex};
     use std::ptr::NonNull;
 
     fn get_object(py: Python<'_>) -> PyObject {
-        // Convenience function for getting a single unique object, using `new_pool` so as to leave
-        // the original pool state unchanged.
-        let pool = unsafe { py.new_pool() };
-        let py = pool.python();
-
-        let obj = py.eval_bound("object()", None, None).unwrap();
-        obj.to_object(py)
+        py.eval_bound("object()", None, None).unwrap().unbind()
     }
 
     fn owned_object_count() -> usize {
@@ -556,6 +564,7 @@ mod tests {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn test_owned() {
         Python::with_gil(|py| {
             let obj = get_object(py);
@@ -581,6 +590,7 @@ mod tests {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn test_owned_nested() {
         Python::with_gil(|py| {
             let obj = get_object(py);
@@ -666,6 +676,7 @@ mod tests {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn test_gil_counts() {
         // Check with_gil and GILPool both increase counts correctly
         let get_gil_count = || GIL_COUNT.with(|c| c.get());
@@ -906,6 +917,7 @@ mod tests {
             unsafe extern "C" fn capsule_drop(capsule: *mut ffi::PyObject) {
                 // This line will implicitly call update_counts
                 // -> and so cause deadlock if update_counts is not handling recursion correctly.
+                #[allow(deprecated)]
                 let pool = GILPool::new();
 
                 // Rebuild obj so that it can be dropped

src/impl_/trampoline.rs

@@ -9,9 +9,11 @@ use std::{
     panic::{self, UnwindSafe},
 };
 
+#[allow(deprecated)]
+use crate::gil::GILPool;
 use crate::{
     callback::PyCallbackOutput, ffi, ffi_ptr_ext::FfiPtrExt, impl_::panic::PanicTrap,
-    methods::IPowModulo, panic::PanicException, types::PyModule, GILPool, Py, PyResult, Python,
+    methods::IPowModulo, panic::PanicException, types::PyModule, Py, PyResult, Python,
 };
 
 #[inline]
@@ -174,6 +176,8 @@ where
     R: PyCallbackOutput,
 {
     let trap = PanicTrap::new("uncaught panic at ffi boundary");
+    // Necessary to construct a pool until PyO3 0.22 when the GIL Refs API is fully disabled
+    #[allow(deprecated)]
     let pool = unsafe { GILPool::new() };
     let py = pool.python();
     let out = panic_result_into_callback_output(
@@ -219,6 +223,8 @@ where
     F: for<'py> FnOnce(Python<'py>) -> PyResult<()> + UnwindSafe,
 {
     let trap = PanicTrap::new("uncaught panic at ffi boundary");
+    // Necessary to construct a pool until PyO3 0.22 when the GIL Refs API is fully disabled
+    #[allow(deprecated)]
     let pool = GILPool::new();
     let py = pool.python();
     if let Err(py_err) = panic::catch_unwind(move || body(py))

src/lib.rs

@@ -319,6 +319,7 @@ pub use crate::conversion::{FromPyPointer, PyTryFrom, PyTryInto};
 pub use crate::err::{
     DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyErrArguments, PyResult, ToPyErr,
 };
+#[allow(deprecated)]
 pub use crate::gil::GILPool;
 #[cfg(not(PyPy))]
 pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter};

src/marker.rs

@@ -118,7 +118,7 @@
 //! [`Py`]: crate::Py
 use crate::err::{self, PyDowncastError, PyErr, PyResult};
 use crate::ffi_ptr_ext::FfiPtrExt;
-use crate::gil::{GILGuard, GILPool, SuspendGIL};
+use crate::gil::{GILGuard, SuspendGIL};
 use crate::impl_::not_send::NotSend;
 use crate::py_result_ext::PyResultExt;
 use crate::type_object::HasPyGilRef;
@@ -127,9 +127,9 @@ use crate::types::{
     PyAny, PyDict, PyEllipsis, PyModule, PyNone, PyNotImplemented, PyString, PyType,
 };
 use crate::version::PythonVersionInfo;
-#[allow(deprecated)]
-use crate::FromPyPointer;
 use crate::{ffi, Bound, IntoPy, Py, PyNativeType, PyObject, PyTypeCheck, PyTypeInfo};
+#[allow(deprecated)]
+use crate::{gil::GILPool, FromPyPointer};
 use std::ffi::{CStr, CString};
 use std::marker::PhantomData;
 use std::os::raw::c_int;
@@ -1053,9 +1053,10 @@ impl<'py> Python<'py> {
         err::error_on_minusone(self, unsafe { ffi::PyErr_CheckSignals() })
     }
 
-    /// Create a new pool for managing PyO3's owned references.
+    /// Create a new pool for managing PyO3's GIL Refs. This has no functional
+    /// use for code which does not use the deprecated GIL Refs API.
     ///
-    /// When this `GILPool` is dropped, all PyO3 owned references created after this `GILPool` will
+    /// When this `GILPool` is dropped, all GIL Refs created after this `GILPool` will
     /// all have their Python reference counts decremented, potentially allowing Python to drop
     /// the corresponding Python objects.
     ///
@@ -1074,6 +1075,7 @@ impl<'py> Python<'py> {
     ///     // Some long-running process like a webserver, which never releases the GIL.
     ///     loop {
     ///         // Create a new pool, so that PyO3 can clear memory at the end of the loop.
+    ///         #[allow(deprecated)]  // `new_pool` is not needed in code not using the GIL Refs API
     ///         let pool = unsafe { py.new_pool() };
     ///
     ///         // It is recommended to *always* immediately set py to the pool's Python, to help
@@ -1108,13 +1110,22 @@ impl<'py> Python<'py> {
     ///
     /// [`.python()`]: crate::GILPool::python
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "code not using the GIL Refs API can safely remove use of `Python::new_pool`"
+        )
+    )]
+    #[allow(deprecated)]
     pub unsafe fn new_pool(self) -> GILPool {
         GILPool::new()
     }
 }
 
 impl Python<'_> {
-    /// Creates a scope using a new pool for managing PyO3's owned references.
+    /// Creates a scope using a new pool for managing PyO3's GIL Refs. This has no functional
+    /// use for code which does not use the deprecated GIL Refs API.
     ///
     /// This is a safe alterantive to [`new_pool`][Self::new_pool] as
     /// it limits the closure to using the new GIL token at the cost of
@@ -1131,6 +1142,7 @@ impl Python<'_> {
     ///     // Some long-running process like a webserver, which never releases the GIL.
     ///     loop {
     ///         // Create a new scope, so that PyO3 can clear memory at the end of the loop.
+    ///         #[allow(deprecated)]  // `with_pool` is not needed in code not using the GIL Refs API
     ///         py.with_pool(|py| {
     ///             // do stuff...
     ///         });
@@ -1167,6 +1179,14 @@ impl Python<'_> {
     /// });
     /// ```
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "code not using the GIL Refs API can safely remove use of `Python::with_pool`"
+        )
+    )]
+    #[allow(deprecated)]
     pub fn with_pool<F, R>(&self, f: F) -> R
     where
         F: for<'py> FnOnce(Python<'py>) -> R + Ungil,

tests/ui/deprecations.rs

@@ -1,4 +1,5 @@
 #![deny(deprecated)]
+#![allow(dead_code)]
 
 use pyo3::prelude::*;
 use pyo3::types::{PyString, PyType};

tests/ui/deprecations.stderr

@@ -1,7 +1,7 @@
 error: use of deprecated constant `pyo3::impl_::deprecations::PYMETHODS_NEW_DEPRECATED_FORM`: use `#[new]` instead of `#[__new__]`
-  --> tests/ui/deprecations.rs:11:7
+  --> tests/ui/deprecations.rs:12:7
    |
-11 |     #[__new__]
+12 |     #[__new__]
    |       ^^^^^^^
    |
 note: the lint level is defined here
@@ -11,45 +11,45 @@ note: the lint level is defined here
    |         ^^^^^^^^^^
 
 error: use of deprecated struct `pyo3::PyCell`: `PyCell` was merged into `Bound`, use that instead; see the migration guide for more info
-  --> tests/ui/deprecations.rs:22:30
+  --> tests/ui/deprecations.rs:23:30
    |
-22 |     fn method_gil_ref(_slf: &PyCell<Self>) {}
+23 |     fn method_gil_ref(_slf: &PyCell<Self>) {}
    |                              ^^^^^^
 
 error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:17:33
+  --> tests/ui/deprecations.rs:18:33
    |
-17 |     fn cls_method_gil_ref(_cls: &PyType) {}
+18 |     fn cls_method_gil_ref(_cls: &PyType) {}
    |                                 ^
 
 error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:22:29
+  --> tests/ui/deprecations.rs:23:29
    |
-22 |     fn method_gil_ref(_slf: &PyCell<Self>) {}
+23 |     fn method_gil_ref(_slf: &PyCell<Self>) {}
    |                             ^
 
 error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:37:43
+  --> tests/ui/deprecations.rs:38:43
    |
-37 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> {
+38 | fn pyfunction_with_module_gil_ref(module: &PyModule) -> PyResult<&str> {
    |                                           ^
 
 error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:47:19
+  --> tests/ui/deprecations.rs:48:19
    |
-47 | fn module_gil_ref(m: &PyModule) -> PyResult<()> {
+48 | fn module_gil_ref(m: &PyModule) -> PyResult<()> {
    |                   ^
 
 error: use of deprecated method `pyo3::methods::Extractor::<T>::extract_gil_ref`: use `&Bound<'_, T>` instead for this function argument
-  --> tests/ui/deprecations.rs:53:57
+  --> tests/ui/deprecations.rs:54:57
    |
-53 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
+54 | fn module_gil_ref_with_explicit_py_arg(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
    |                                                         ^
 
 error: use of deprecated method `pyo3::methods::Extractor::<pyo3::Python<'_>>::is_python`: use `wrap_pyfunction_bound!` instead
-  --> tests/ui/deprecations.rs:78:13
+  --> tests/ui/deprecations.rs:79:13
    |
-78 |     let _ = wrap_pyfunction!(double, py);
+79 |     let _ = wrap_pyfunction!(double, py);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this error originates in the macro `wrap_pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)