deprecate `GILPool` (#3947)

* deprecate `GILPool`

* review: adamreichold

* fix deprecation warnings in tests
This commit is contained in:
David Hewitt 2024-03-15 10:25:27 +00:00 committed by GitHub
parent 5c86dc35c1
commit dcba984b51
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 88 additions and 33 deletions

View File

@ -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

View File

@ -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.

View File

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

View File

@ -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,6 +361,13 @@ 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`.
@ -365,6 +375,7 @@ pub struct GILPool {
_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

View File

@ -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))

View File

@ -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};

View File

@ -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,

View File

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

View File

@ -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)