diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbd7ff4a73a..392cfc213b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -97,7 +97,7 @@ jobs: with: toolchain: ${{ matrix.rust }} targets: ${{ matrix.platform.rust-target }} - components: clippy + components: clippy,rust-src - uses: actions/setup-python@v4 with: architecture: ${{ matrix.platform.python-architecture }} diff --git a/Cargo.toml b/Cargo.toml index f8e285c1c64..5ee6144bfad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,8 +91,7 @@ abi3-py311 = ["abi3", "pyo3-build-config/abi3-py311", "pyo3-ffi/abi3-py311"] # Automatically generates `python3.dll` import libraries for Windows targets. generate-import-lib = ["pyo3-ffi/generate-import-lib"] -# Changes `Python::with_gil` and `Python::acquire_gil` to automatically initialize the -# Python interpreter if needed. +# Changes `Python::with_gil` to automatically initialize the Python interpreter if needed. auto-initialize = [] # Optimizes PyObject to Vec conversion and so on. diff --git a/guide/src/features.md b/guide/src/features.md index d9a051e4641..fc3b8eebf8c 100644 --- a/guide/src/features.md +++ b/guide/src/features.md @@ -45,7 +45,7 @@ section for further detail. ### `auto-initialize` -This feature changes [`Python::with_gil`]({{#PYO3_DOCS_URL}}/pyo3/struct.Python.html#method.with_gil) and [`Python::acquire_gil`]({{#PYO3_DOCS_URL}}/pyo3/struct.Python.html#method.acquire_gil) to automatically initialize a Python interpreter (by calling [`prepare_freethreaded_python`]({{#PYO3_DOCS_URL}}/pyo3/fn.prepare_freethreaded_python.html)) if needed. +This feature changes [`Python::with_gil`]({{#PYO3_DOCS_URL}}/pyo3/struct.Python.html#method.with_gil) to automatically initialize a Python interpreter (by calling [`prepare_freethreaded_python`]({{#PYO3_DOCS_URL}}/pyo3/fn.prepare_freethreaded_python.html)) if needed. If you do not enable this feature, you should call `pyo3::prepare_freethreaded_python()` before attempting to call any other Python APIs. diff --git a/guide/src/memory.md b/guide/src/memory.md index 0b430495e4d..79a3abcb011 100644 --- a/guide/src/memory.md +++ b/guide/src/memory.md @@ -35,10 +35,9 @@ Python::with_gil(|py| -> PyResult<()> { # } ``` -Internally, calling `Python::with_gil()` or `Python::acquire_gil()` creates a -`GILPool` which owns the memory pointed to by the reference. In the example -above, the lifetime of the reference `hello` is bound to the `GILPool`. When -the `with_gil()` closure ends or the `GILGuard` from `acquire_gil()` is dropped, +Internally, calling `Python::with_gil()` creates a `GILPool` which owns the +memory pointed to by the reference. In the example above, the lifetime of the +reference `hello` is bound to the `GILPool`. When the `with_gil()` closure ends the `GILPool` is also dropped and the Python reference counts of the variables it owns are decreased, releasing them to the Python garbage collector. Most of the time we don't have to think about this, but consider the following: diff --git a/guide/src/migration.md b/guide/src/migration.md index 1b6b58439dc..60441363a16 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -40,6 +40,78 @@ After, the same code will print `ValueError: original error message`, which is m However, if the `anyhow::Error` or `eyre::Report` has a source, then the original exception will still be wrapped in a `PyRuntimeError`. +### The deprecated `Python::acquire_gil` was removed and `Python::with_gil` must be used instead + +While the API provided by [`Python::acquire_gil`](https://docs.rs/pyo3/0.18.3/pyo3/marker/struct.Python.html#method.acquire_gil) seems convenient, it is somewhat brittle as the design of the GIL token [`Python`](https://docs.rs/pyo3/0.18.3/pyo3/marker/struct.Python.html) relies on proper nesting and panics if not used correctly, e.g. + +```rust,ignore +# #![allow(dead_code, deprecated)] +# use pyo3::prelude::*; + +#[pyclass] +struct SomeClass {} + +struct ObjectAndGuard { + object: Py, + guard: GILGuard, +} + +impl ObjectAndGuard { + fn new() -> Self { + let guard = Python::acquire_gil(); + let object = Py::new(guard.python(), SomeClass {}).unwrap(); + + Self { object, guard } + } +} + +let first = ObjectAndGuard::new(); +let second = ObjectAndGuard::new(); +// Panics because the guard within `second` is still alive. +drop(first); +drop(second); +``` + +The replacement is [`Python::with_gil`]() which is more cumbersome but enforces the proper nesting by design, e.g. + +```rust +# #![allow(dead_code)] +# use pyo3::prelude::*; + +#[pyclass] +struct SomeClass {} + +struct Object { + object: Py, +} + +impl Object { + fn new(py: Python<'_>) -> Self { + let object = Py::new(py, SomeClass {}).unwrap(); + + Self { object } + } +} + +// It either forces us to release the GIL before aquiring it again. +let first = Python::with_gil(|py| Object::new(py)); +let second = Python::with_gil(|py| Object::new(py)); +drop(first); +drop(second); + +// Or it ensure releasing the inner lock before the outer one. +Python::with_gil(|py| { + let first = Object::new(py); + let second = Python::with_gil(|py| { + Object::new(py) + }); + drop(first); + drop(second); +}); +``` + +Furthermore, `Python::acquire_gil` provides ownership of a `GILGuard` which can be freely stored and passed around. This is usually not helpful as it may keep the lock held for a long time thereby blocking progress in other parts of the program. Due to the generative lifetime attached to the GIL token supplied by `Python::with_gil`, the problem is avoided as the GIL token can only be passed down the call chain. Often, this issue can also be avoided entirely as any GIL-bound reference `&'py PyAny` implies access to a GIL token `Python<'py>` via the [`PyAny::py`](https://docs.rs/pyo3/latest/pyo3/types/struct.PyAny.html#method.py) method. + ## from 0.17.* to 0.18 ### Required arguments after `Option<_>` arguments will no longer be automatically inferred diff --git a/newsfragments/2981.removed.md b/newsfragments/2981.removed.md new file mode 100644 index 00000000000..9df28751ee4 --- /dev/null +++ b/newsfragments/2981.removed.md @@ -0,0 +1 @@ +Remove all functionality deprecated in PyO3 0.17, most prominently `Python::acquire_gil` is replaced by `Python::with_gil`. diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 42ae7029ce5..b79af4b9e51 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -2,14 +2,6 @@ use pyo3::prelude::*; #[pyfunction] fn issue_219() { - // issue 219: acquiring GIL inside #[pyfunction] deadlocks. - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let _py = gil.python(); -} - -#[pyfunction] -fn issue_219_2() { // issue 219: acquiring GIL inside #[pyfunction] deadlocks. Python::with_gil(|_| {}); } @@ -17,6 +9,5 @@ fn issue_219_2() { #[pymodule] pub fn misc(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_function(wrap_pyfunction!(issue_219, m)?)?; - m.add_function(wrap_pyfunction!(issue_219_2, m)?)?; Ok(()) } diff --git a/pytests/tests/test_misc.py b/pytests/tests/test_misc.py index 0449b418c6d..8dfd06ba298 100644 --- a/pytests/tests/test_misc.py +++ b/pytests/tests/test_misc.py @@ -8,7 +8,6 @@ def test_issue_219(): # Should not deadlock pyo3_pytests.misc.issue_219() - pyo3_pytests.misc.issue_219_2() @pytest.mark.skipif( diff --git a/src/conversion.rs b/src/conversion.rs index 93f2c362337..c5bda16fd7c 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -135,34 +135,6 @@ pub trait ToPyObject { fn to_object(&self, py: Python<'_>) -> PyObject; } -/// A deprecated conversion trait which relied on the unstable `specialization` feature -/// of the Rust language. -#[deprecated( - since = "0.17.0", - note = "this trait is no longer used by PyO3, use ToPyObject or IntoPy" -)] -pub trait ToBorrowedObject: ToPyObject { - /// Converts self into a Python object and calls the specified closure - /// on the native FFI pointer underlying the Python object. - /// - /// May be more efficient than `to_object` because it does not need - /// to touch any reference counts when the input object already is a Python object. - fn with_borrowed_ptr(&self, py: Python<'_>, f: F) -> R - where - F: FnOnce(*mut ffi::PyObject) -> R, - { - let ptr = self.to_object(py).into_ptr(); - let result = f(ptr); - unsafe { - ffi::Py_XDECREF(ptr); - } - result - } -} - -#[allow(deprecated)] -impl ToBorrowedObject for T where T: ToPyObject {} - /// Defines a conversion from a Rust type to a Python object. /// /// It functions similarly to std's [`Into`](std::convert::Into) trait, diff --git a/src/gil.rs b/src/gil.rs index 217261dc184..aa4ed2e7825 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -6,11 +6,7 @@ use crate::impl_::not_send::{NotSend, NOT_SEND}; use crate::{ffi, Python}; use parking_lot::{const_mutex, Mutex, Once}; use std::cell::{Cell, RefCell}; -use std::{ - mem::{self, ManuallyDrop}, - ptr::NonNull, - sync::atomic, -}; +use std::{mem, ptr::NonNull, sync::atomic}; static START: Once = Once::new(); @@ -139,25 +135,9 @@ where } /// RAII type that represents the Global Interpreter Lock acquisition. -/// -/// Users are strongly encouraged to use [`Python::with_gil`](struct.Python.html#method.with_gil) -/// instead of directly constructing this type. -/// See [`Python::acquire_gil`](struct.Python.html#method.acquire_gil) for more. -/// -/// # Examples -/// ``` -/// use pyo3::Python; -/// -/// { -/// #[allow(deprecated)] -/// let gil_guard = Python::acquire_gil(); -/// let py = gil_guard.python(); -/// } // GIL is released when gil_guard is dropped -/// ``` -#[must_use] -pub struct GILGuard { +struct GILGuard { gstate: ffi::PyGILState_STATE, - pool: ManuallyDrop>, + pool: Option, _not_send: NotSend, } @@ -168,11 +148,11 @@ impl GILGuard { unsafe { Python::assume_gil_acquired() } } - /// PyO3 internal API for acquiring the GIL. The public API is Python::acquire_gil. + /// PyO3 internal API for acquiring the GIL. The public API is Python::with_gil. /// /// If PyO3 does not yet have a `GILPool` for tracking owned PyObject references, then this new /// `GILGuard` will also contain a `GILPool`. - pub(crate) fn acquire() -> GILGuard { + fn acquire() -> GILGuard { // Maybe auto-initialize the GIL: // - If auto-initialize feature set and supported, try to initialize the interpreter. // - If the auto-initialize feature is set but unsupported, emit hard errors only when the @@ -231,7 +211,7 @@ impl GILGuard { GILGuard { gstate, - pool: ManuallyDrop::new(pool), + pool, _not_send: NOT_SEND, } } @@ -240,28 +220,12 @@ impl GILGuard { /// The Drop implementation for `GILGuard` will release the GIL. impl Drop for GILGuard { fn drop(&mut self) { - // First up, try to detect if the order of destruction is correct. - #[allow(clippy::manual_assert)] - let _ = GIL_COUNT.try_with(|c| { - if self.gstate == ffi::PyGILState_STATE::PyGILState_UNLOCKED && c.get() != 1 { - // XXX: this panic commits to leaking all objects in the pool as well as - // potentially meaning the GIL never releases. Perhaps should be an abort? - // Unfortunately abort UX is much worse than panic. - panic!("The first GILGuard acquired must be the last one dropped."); - } - }); - - // If this GILGuard doesn't own a pool, then need to decrease the count after dropping - // all objects from the pool. - let should_decrement = self.pool.is_none(); - // Drop the objects in the pool before attempting to release the thread state - unsafe { - ManuallyDrop::drop(&mut self.pool); - } - - if should_decrement { - decrement_gil_count(); + if let Some(pool) = self.pool.take() { + drop(pool) + } else { + // This GILGuard didn't own a pool, need to decrease the count manually. + decrement_gil_count() } unsafe { @@ -554,62 +518,60 @@ mod tests { #[test] fn test_owned() { - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); - let obj = get_object(py); - let obj_ptr = obj.as_ptr(); - // Ensure that obj does not get freed - let _ref = obj.clone_ref(py); + Python::with_gil(|py| { + let obj = get_object(py); + let obj_ptr = obj.as_ptr(); + // Ensure that obj does not get freed + let _ref = obj.clone_ref(py); - unsafe { - { - let pool = py.new_pool(); - gil::register_owned(pool.python(), NonNull::new_unchecked(obj.into_ptr())); + unsafe { + { + let pool = py.new_pool(); + gil::register_owned(pool.python(), NonNull::new_unchecked(obj.into_ptr())); - assert_eq!(owned_object_count(), 1); - assert_eq!(ffi::Py_REFCNT(obj_ptr), 2); - } - { - let _pool = py.new_pool(); - assert_eq!(owned_object_count(), 0); - assert_eq!(ffi::Py_REFCNT(obj_ptr), 1); + assert_eq!(owned_object_count(), 1); + assert_eq!(ffi::Py_REFCNT(obj_ptr), 2); + } + { + let _pool = py.new_pool(); + assert_eq!(owned_object_count(), 0); + assert_eq!(ffi::Py_REFCNT(obj_ptr), 1); + } } - } + }) } #[test] fn test_owned_nested() { - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); - let obj = get_object(py); - // Ensure that obj does not get freed - let _ref = obj.clone_ref(py); - let obj_ptr = obj.as_ptr(); - - unsafe { - { - let _pool = py.new_pool(); - assert_eq!(owned_object_count(), 0); - - gil::register_owned(py, NonNull::new_unchecked(obj.into_ptr())); + Python::with_gil(|py| { + let obj = get_object(py); + // Ensure that obj does not get freed + let _ref = obj.clone_ref(py); + let obj_ptr = obj.as_ptr(); - assert_eq!(owned_object_count(), 1); - assert_eq!(ffi::Py_REFCNT(obj_ptr), 2); + unsafe { { let _pool = py.new_pool(); - let obj = get_object(py); + assert_eq!(owned_object_count(), 0); + gil::register_owned(py, NonNull::new_unchecked(obj.into_ptr())); - assert_eq!(owned_object_count(), 2); + + assert_eq!(owned_object_count(), 1); + assert_eq!(ffi::Py_REFCNT(obj_ptr), 2); + { + let _pool = py.new_pool(); + let obj = get_object(py); + gil::register_owned(py, NonNull::new_unchecked(obj.into_ptr())); + assert_eq!(owned_object_count(), 2); + } + assert_eq!(owned_object_count(), 1); + } + { + assert_eq!(owned_object_count(), 0); + assert_eq!(ffi::Py_REFCNT(obj_ptr), 1); } - assert_eq!(owned_object_count(), 1); - } - { - assert_eq!(owned_object_count(), 0); - assert_eq!(ffi::Py_REFCNT(obj_ptr), 1); } - } + }); } #[test] @@ -666,59 +628,53 @@ mod tests { #[test] fn test_gil_counts() { - // Check GILGuard and GILPool both increase counts correctly + // Check with_gil and GILPool both increase counts correctly let get_gil_count = || GIL_COUNT.with(|c| c.get()); assert_eq!(get_gil_count(), 0); - #[allow(deprecated)] - let gil = Python::acquire_gil(); - assert_eq!(get_gil_count(), 1); - - assert_eq!(get_gil_count(), 1); - let pool = unsafe { GILPool::new() }; - assert_eq!(get_gil_count(), 2); + Python::with_gil(|_| { + assert_eq!(get_gil_count(), 1); - let pool2 = unsafe { GILPool::new() }; - assert_eq!(get_gil_count(), 3); + let pool = unsafe { GILPool::new() }; + assert_eq!(get_gil_count(), 2); - drop(pool); - assert_eq!(get_gil_count(), 2); + let pool2 = unsafe { GILPool::new() }; + assert_eq!(get_gil_count(), 3); - #[allow(deprecated)] - let gil2 = Python::acquire_gil(); - assert_eq!(get_gil_count(), 3); + drop(pool); + assert_eq!(get_gil_count(), 2); - drop(gil2); - assert_eq!(get_gil_count(), 2); - - drop(pool2); - assert_eq!(get_gil_count(), 1); + Python::with_gil(|_| { + // nested with_gil doesn't update gil count + assert_eq!(get_gil_count(), 2); + }); + assert_eq!(get_gil_count(), 2); - drop(gil); + drop(pool2); + assert_eq!(get_gil_count(), 1); + }); assert_eq!(get_gil_count(), 0); } #[test] fn test_allow_threads() { - // allow_threads should temporarily release GIL in PyO3's internal tracking too. - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); + assert!(!gil_is_acquired()); - assert!(gil_is_acquired()); + Python::with_gil(|py| { + assert!(gil_is_acquired()); - py.allow_threads(move || { - assert!(!gil_is_acquired()); + py.allow_threads(move || { + assert!(!gil_is_acquired()); - #[allow(deprecated)] - let gil = Python::acquire_gil(); - assert!(gil_is_acquired()); + Python::with_gil(|_| assert!(gil_is_acquired())); - drop(gil); - assert!(!gil_is_acquired()); + assert!(!gil_is_acquired()); + }); + + assert!(gil_is_acquired()); }); - assert!(gil_is_acquired()); + assert!(!gil_is_acquired()); } #[test] @@ -740,32 +696,25 @@ mod tests { #[test] fn dropping_gil_does_not_invalidate_references() { // Acquiring GIL for the second time should be safe - see #864 - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); - - #[allow(deprecated)] - let gil2 = Python::acquire_gil(); - let obj = py.eval("object()", None, None).unwrap(); - drop(gil2); + Python::with_gil(|py| { + let obj = Python::with_gil(|_| py.eval("object()", None, None).unwrap()); - // After gil2 drops, obj should still have a reference count of one - assert_eq!(obj.get_refcnt(), 1); + // After gil2 drops, obj should still have a reference count of one + assert_eq!(obj.get_refcnt(), 1); + }) } #[test] fn test_clone_with_gil() { - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); - - let obj = get_object(py); - let count = obj.get_refcnt(py); + Python::with_gil(|py| { + let obj = get_object(py); + let count = obj.get_refcnt(py); - // Cloning with the GIL should increase reference count immediately - #[allow(clippy::redundant_clone)] - let c = obj.clone(); - assert_eq!(count + 1, c.get_refcnt(py)); + // Cloning with the GIL should increase reference count immediately + #[allow(clippy::redundant_clone)] + let c = obj.clone(); + assert_eq!(count + 1, c.get_refcnt(py)); + }) } #[cfg(not(target_arch = "wasm32"))] @@ -912,11 +861,9 @@ mod tests { // update_counts can run arbitrary Python code during Py_DECREF. // if the locking is implemented incorrectly, it will deadlock. - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let obj = get_object(gil.python()); + Python::with_gil(|py| { + let obj = get_object(py); - unsafe { 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. @@ -930,12 +877,14 @@ mod tests { } let ptr = obj.into_ptr(); - let capsule = ffi::PyCapsule_New(ptr as _, std::ptr::null(), Some(capsule_drop)); + + let capsule = + unsafe { ffi::PyCapsule_New(ptr as _, std::ptr::null(), Some(capsule_drop)) }; POOL.register_decref(NonNull::new(capsule).unwrap()); // Updating the counts will call decref on the capsule, which calls capsule_drop - POOL.update_counts(gil.python()) - } + POOL.update_counts(py); + }) } } diff --git a/src/lib.rs b/src/lib.rs index 1a33a135476..ded43370148 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,8 +79,8 @@ //! //! - `abi3`: Restricts PyO3's API to a subset of the full Python API which is guaranteed by //! [PEP 384] to be forward-compatible with future Python versions. -//! - `auto-initialize`: Changes [`Python::with_gil`] and [`Python::acquire_gil`] to automatically -//! initialize the Python interpreter if needed. +//! - `auto-initialize`: Changes [`Python::with_gil`] to automatically initialize the Python +//! interpreter if needed. //! - `extension-module`: This will tell the linker to keep the Python symbols unresolved, so that //! your module can also be used with statically linked Python interpreters. Use this feature when //! building an extension module. @@ -300,16 +300,14 @@ //! [Features chapter of the guide]: https://pyo3.rs/latest/features.html#features-reference "Features Reference - PyO3 user guide" //! [`Ungil`]: crate::marker::Ungil pub use crate::class::*; -#[allow(deprecated)] -pub use crate::conversion::ToBorrowedObject; pub use crate::conversion::{ AsPyPointer, FromPyObject, FromPyPointer, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto, ToPyObject, }; pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult}; +pub use crate::gil::GILPool; #[cfg(not(PyPy))] pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter}; -pub use crate::gil::{GILGuard, GILPool}; pub use crate::instance::{Py, PyNativeType, PyObject}; pub use crate::marker::Python; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; diff --git a/src/marker.rs b/src/marker.rs index b9c5759e8f5..bd30d3c3396 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -120,7 +120,7 @@ //! [`Rc`]: std::rc::Rc //! [`Py`]: crate::Py use crate::err::{self, PyDowncastError, PyErr, PyResult}; -use crate::gil::{self, GILGuard, GILPool, SuspendGIL}; +use crate::gil::{self, EnsureGIL, GILPool, SuspendGIL}; use crate::impl_::not_send::NotSend; use crate::types::{PyAny, PyDict, PyModule, PyString, PyType}; use crate::version::PythonVersionInfo; @@ -273,12 +273,16 @@ mod negative_impls { /// [`Py::clone_ref`]: crate::Py::clone_ref /// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory #[derive(Copy, Clone)] -pub struct Python<'py>(PhantomData<(&'py GILGuard, NotSend)>); +pub struct Python<'py>(PhantomData<(&'py EnsureGIL, NotSend)>); impl Python<'_> { /// Acquires the global interpreter lock, allowing access to the Python interpreter. The /// provided closure `F` will be executed with the acquired `Python` marker token. /// + /// If implementing [`#[pymethods]`](crate::pymethods) or [`#[pyfunction]`](crate::pyfunction), + /// declare `py: Python` as an argument. PyO3 will pass in the token to grant access to the GIL + /// context in which the function is running, avoiding the need to call `with_gil`. + /// /// If the [`auto-initialize`] feature is enabled and the Python runtime is not already /// initialized, this function will initialize it. See #[cfg_attr( @@ -353,52 +357,6 @@ impl Python<'_> { } impl<'py> Python<'py> { - /// Acquires the global interpreter lock, allowing access to the Python interpreter. - /// - /// If the [`auto-initialize`] feature is enabled and the Python runtime is not already - /// initialized, this function will initialize it. See - #[cfg_attr( - not(PyPy), - doc = "[`prepare_freethreaded_python`](crate::prepare_freethreaded_python)" - )] - #[cfg_attr(PyPy, doc = "`prepare_freethreaded_python`")] - /// for details. - /// - /// Most users should not need to use this API directly, and should prefer one of two options: - /// 1. If implementing [`#[pymethods]`](crate::pymethods) or [`#[pyfunction]`](crate::pyfunction), declare `py: Python` as an argument. - /// PyO3 will pass in the token to grant access to the GIL context in which the function is running. - /// 2. Use [`Python::with_gil`] to run a closure with the GIL, acquiring only if needed. - /// - /// # Panics - /// - /// - If the [`auto-initialize`] feature is not enabled and the Python interpreter is not - /// initialized. - /// - If multiple [`GILGuard`]s are not dropped in in the reverse order of acquisition, PyO3 - /// may panic. It is recommended to use [`Python::with_gil`] instead to avoid this. - /// - /// # Notes - /// - /// The return type from this function, [`GILGuard`], is implemented as a RAII guard - /// around [`PyGILState_Ensure`]. This means that multiple `acquire_gil()` calls are - /// allowed, and will not deadlock. However, [`GILGuard`]s must be dropped in the reverse order - /// to acquisition. If PyO3 detects this order is not maintained, it will panic when the out-of-order drop occurs. - /// - /// # Deprecation - /// - /// This API has been deprecated for several reasons: - /// - GIL drop order tracking has turned out to be [error prone](https://github.com/PyO3/pyo3/issues/1683). - /// With a scoped API like `Python::with_gil`, these are always dropped in the correct order. - /// - It promotes passing and keeping the GILGuard around, which is almost always not what you actually want. - /// - /// [`PyGILState_Ensure`]: crate::ffi::PyGILState_Ensure - /// [`auto-initialize`]: https://pyo3.rs/main/features.html#auto-initialize - #[inline] - // Once removed, we can remove GILGuard's drop tracking. - #[deprecated(since = "0.17.0", note = "prefer Python::with_gil")] - pub fn acquire_gil() -> GILGuard { - GILGuard::acquire() - } - /// Temporarily releases the GIL, thus allowing other Python threads to run. The GIL will be /// reacquired when `F`'s scope ends. /// @@ -820,8 +778,8 @@ impl<'py> Python<'py> { /// all have their Python reference counts decremented, potentially allowing Python to drop /// the corresponding Python objects. /// - /// Typical usage of PyO3 will not need this API, as [`Python::with_gil`] and - /// [`Python::acquire_gil`] automatically create a `GILPool` where appropriate. + /// Typical usage of PyO3 will not need this API, as [`Python::with_gil`] automatically creates + /// a `GILPool` where appropriate. /// /// Advanced uses of PyO3 which perform long-running tasks which never free the GIL may need /// to use this API to clear memory, as PyO3 usually does not clear memory until the GIL is diff --git a/src/prelude.rs b/src/prelude.rs index 6ffc470444e..a110ea5c1f5 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -14,7 +14,6 @@ pub use crate::conversion::{ FromPyObject, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto, ToPyObject, }; pub use crate::err::{PyErr, PyResult}; -pub use crate::gil::GILGuard; pub use crate::instance::{Py, PyObject}; pub use crate::marker::Python; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; diff --git a/src/type_object.rs b/src/type_object.rs index a4a84d6b7d8..82e9750316c 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -62,20 +62,6 @@ pub unsafe trait PyTypeInfo: Sized { } } -/// Legacy trait which previously held the `type_object` method now found on `PyTypeInfo`. -/// -/// # Safety -/// -/// This trait used to have stringent safety requirements, but they are now irrelevant as it is deprecated. -#[deprecated( - since = "0.17.0", - note = "PyTypeObject::type_object was moved to PyTypeInfo::type_object" -)] -pub unsafe trait PyTypeObject: PyTypeInfo {} - -#[allow(deprecated)] -unsafe impl PyTypeObject for T {} - #[inline] pub(crate) unsafe fn get_tp_alloc(tp: *mut ffi::PyTypeObject) -> Option { #[cfg(not(Py_LIMITED_API))] @@ -104,23 +90,3 @@ pub(crate) unsafe fn get_tp_free(tp: *mut ffi::PyTypeObject) -> ffi::freefunc { std::mem::transmute(ptr) } } - -#[cfg(test)] -mod tests { - #[test] - #[allow(deprecated)] - fn test_deprecated_type_object() { - // Even though PyTypeObject is deprecated, simple usages of it as a trait bound should continue to work. - use super::PyTypeObject; - use crate::types::{PyList, PyType}; - use crate::Python; - - fn get_type_object(py: Python<'_>) -> &PyType { - T::type_object(py) - } - - Python::with_gil(|py| { - assert!(get_type_object::(py).is(::type_object(py))) - }); - } -} diff --git a/src/types/capsule.rs b/src/types/capsule.rs index 4136e52d0b7..7da59e735e3 100644 --- a/src/types/capsule.rs +++ b/src/types/capsule.rs @@ -198,12 +198,6 @@ impl PyCapsule { Ok(ctx) } - /// Deprecated form of `.context()`. - #[deprecated(since = "0.17.0", note = "replaced with .context()")] - pub fn get_context(&self, _: Python<'_>) -> PyResult<*mut c_void> { - self.context() - } - /// Obtains a reference to the value of this capsule. /// /// # Safety diff --git a/tests/test_sequence.rs b/tests/test_sequence.rs index 6e8395999b7..8dcbbe19d8c 100644 --- a/tests/test_sequence.rs +++ b/tests/test_sequence.rs @@ -319,22 +319,20 @@ fn test_option_list_get() { #[test] fn sequence_is_not_mapping() { - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let py = gil.python(); - - let list = PyCell::new( - py, - OptionList { - items: vec![Some(1), None], - }, - ) - .unwrap(); + Python::with_gil(|py| { + let list = PyCell::new( + py, + OptionList { + items: vec![Some(1), None], + }, + ) + .unwrap(); - PySequence::register::(py).unwrap(); + PySequence::register::(py).unwrap(); - assert!(list.as_ref().downcast::().is_err()); - assert!(list.as_ref().downcast::().is_ok()); + assert!(list.as_ref().downcast::().is_err()); + assert!(list.as_ref().downcast::().is_ok()); + }) } #[test] diff --git a/tests/ui/not_send.stderr b/tests/ui/not_send.stderr index 18547a10f80..a1a55db338f 100644 --- a/tests/ui/not_send.stderr +++ b/tests/ui/not_send.stderr @@ -9,8 +9,8 @@ error[E0277]: `*mut pyo3::Python<'static>` cannot be shared between threads safe = help: within `pyo3::Python<'_>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>` = note: required because it appears within the type `PhantomData<*mut Python<'static>>` = note: required because it appears within the type `NotSend` - = note: required because it appears within the type `(&GILGuard, NotSend)` - = note: required because it appears within the type `PhantomData<(&GILGuard, NotSend)>` + = note: required because it appears within the type `(&EnsureGIL, NotSend)` + = note: required because it appears within the type `PhantomData<(&EnsureGIL, NotSend)>` = note: required because it appears within the type `Python<'_>` = note: required for `&pyo3::Python<'_>` to implement `Send` note: required because it's used within this closure diff --git a/tests/ui/pyclass_send.rs b/tests/ui/pyclass_send.rs index a46622b74dd..533302740d7 100644 --- a/tests/ui/pyclass_send.rs +++ b/tests/ui/pyclass_send.rs @@ -3,23 +3,24 @@ use std::rc::Rc; #[pyclass] struct NotThreadSafe { - data: Rc + data: Rc, } fn main() { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let obj = PyCell::new(py, NotThreadSafe { data: Rc::new(5) }).unwrap().to_object(py); - drop(gil); + let obj = Python::with_gil(|py| { + PyCell::new(py, NotThreadSafe { data: Rc::new(5) }) + .unwrap() + .to_object(py) + }); std::thread::spawn(move || { - let gil = Python::acquire_gil(); - let py = gil.python(); - - // Uh oh, moved Rc to a new thread! - let c: &PyCell = obj.as_ref(py).downcast().unwrap(); + Python::with_gil(|py| { + // Uh oh, moved Rc to a new thread! + let c: &PyCell = obj.as_ref(py).downcast().unwrap(); - assert_eq!(*c.borrow().data, 5); - }).join().unwrap(); + assert_eq!(*c.borrow().data, 5); + }) + }) + .join() + .unwrap(); } diff --git a/tests/ui/wrong_aspyref_lifetimes.rs b/tests/ui/wrong_aspyref_lifetimes.rs index 98d8adbf4fa..5cd8a2d3cb6 100644 --- a/tests/ui/wrong_aspyref_lifetimes.rs +++ b/tests/ui/wrong_aspyref_lifetimes.rs @@ -1,11 +1,10 @@ use pyo3::{types::PyDict, Py, Python}; fn main() { - #[allow(deprecated)] - let gil = Python::acquire_gil(); - let dict: Py = PyDict::new(gil.python()).into(); - let dict: &PyDict = dict.as_ref(gil.python()); - drop(gil); + let dict: Py = Python::with_gil(|py| PyDict::new(py).into()); + + // Should not be able to get access to Py contents outside of with_gil. + let dict: &PyDict = Python::with_gil(|py| dict.as_ref(py)); let _py: Python = dict.py(); // Obtain a Python<'p> without GIL. } diff --git a/tests/ui/wrong_aspyref_lifetimes.stderr b/tests/ui/wrong_aspyref_lifetimes.stderr index ad1b2c6d565..def6f94c02d 100644 --- a/tests/ui/wrong_aspyref_lifetimes.stderr +++ b/tests/ui/wrong_aspyref_lifetimes.stderr @@ -1,13 +1,8 @@ -error[E0505]: cannot move out of `gil` because it is borrowed - --> tests/ui/wrong_aspyref_lifetimes.rs:8:10 - | -5 | let gil = Python::acquire_gil(); - | --- binding `gil` declared here -6 | let dict: Py = PyDict::new(gil.python()).into(); -7 | let dict: &PyDict = dict.as_ref(gil.python()); - | ------------ borrow of `gil` occurs here -8 | drop(gil); - | ^^^ move out of `gil` occurs here -9 | -10 | let _py: Python = dict.py(); // Obtain a Python<'p> without GIL. - | --------- borrow later used here +error: lifetime may not live long enough + --> tests/ui/wrong_aspyref_lifetimes.rs:7:47 + | +7 | let dict: &PyDict = Python::with_gil(|py| dict.as_ref(py)); + | --- ^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` + | | | + | | return type of closure is &'2 PyDict + | has type `pyo3::Python<'1>`