From dd24c9ea7173812e138f581e3a0bb15b5cffa599 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Wed, 22 Feb 2023 21:46:42 +0000 Subject: [PATCH] remove `Python::acquire_gil` --- Cargo.toml | 3 +- guide/src/features.md | 2 +- guide/src/memory.md | 7 +- guide/src/migration.md | 2 +- pytests/src/misc.rs | 9 - pytests/tests/test_misc.py | 1 - src/gil.rs | 253 ++++++++++-------------- src/lib.rs | 6 +- src/marker.rs | 58 +----- src/prelude.rs | 1 - tests/test_sequence.rs | 26 ++- tests/ui/not_send.stderr | 4 +- tests/ui/pyclass_send.rs | 27 +-- tests/ui/wrong_aspyref_lifetimes.rs | 9 +- tests/ui/wrong_aspyref_lifetimes.stderr | 21 +- 15 files changed, 158 insertions(+), 271 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f8e285c1..5ee6144b 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 d9a051e4..fc3b8eeb 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 0b430495..79a3abcb 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 210dd075..60441363 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -44,7 +44,7 @@ However, if the `anyhow::Error` or `eyre::Report` has a source, then the origina 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,should_panic +```rust,ignore # #![allow(dead_code, deprecated)] # use pyo3::prelude::*; diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 42ae7029..b79af4b9 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 0449b418..8dfd06ba 100644 --- a/pytests/tests/test_misc.py +++ b/pytests/tests/test_misc.py @@ -8,7 +8,6 @@ import pytest 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/gil.rs b/src/gil.rs index 217261dc..aa4ed2e7 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); + 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); + } } - { - 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(); + 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(); - unsafe { - { - let _pool = py.new_pool(); - assert_eq!(owned_object_count(), 0); - - gil::register_owned(py, NonNull::new_unchecked(obj.into_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); + Python::with_gil(|_| { + assert_eq!(get_gil_count(), 1); - assert_eq!(get_gil_count(), 1); - let pool = unsafe { GILPool::new() }; - assert_eq!(get_gil_count(), 2); + let pool = unsafe { GILPool::new() }; + assert_eq!(get_gil_count(), 2); - let pool2 = unsafe { GILPool::new() }; - assert_eq!(get_gil_count(), 3); + let pool2 = unsafe { GILPool::new() }; + assert_eq!(get_gil_count(), 3); - drop(pool); - assert_eq!(get_gil_count(), 2); + drop(pool); + assert_eq!(get_gil_count(), 2); - #[allow(deprecated)] - let gil2 = Python::acquire_gil(); - assert_eq!(get_gil_count(), 3); + Python::with_gil(|_| { + // nested with_gil doesn't update gil count + assert_eq!(get_gil_count(), 2); + }); + assert_eq!(get_gil_count(), 2); - drop(gil2); - assert_eq!(get_gil_count(), 2); - - drop(pool2); - assert_eq!(get_gil_count(), 1); - - 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()); - - py.allow_threads(move || { - assert!(!gil_is_acquired()); - - #[allow(deprecated)] - let gil = Python::acquire_gil(); + Python::with_gil(|py| { assert!(gil_is_acquired()); - drop(gil); - assert!(!gil_is_acquired()); + py.allow_threads(move || { + assert!(!gil_is_acquired()); + + Python::with_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(); + Python::with_gil(|py| { + let obj = Python::with_gil(|_| py.eval("object()", None, None).unwrap()); - #[allow(deprecated)] - let gil2 = Python::acquire_gil(); - let obj = py.eval("object()", None, None).unwrap(); - drop(gil2); - - // 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(); + Python::with_gil(|py| { + let obj = get_object(py); + let count = obj.get_refcnt(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 1a33a135..f66bf1d1 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. @@ -307,9 +307,9 @@ pub use crate::conversion::{ 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 b9c5759e..bd30d3c3 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 6ffc4704..a110ea5c 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/tests/test_sequence.rs b/tests/test_sequence.rs index 6e839599..8dcbbe19 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(); + Python::with_gil(|py| { + let list = PyCell::new( + py, + OptionList { + items: vec![Some(1), None], + }, + ) + .unwrap(); - 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 18547a10..a1a55db3 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 a46622b7..53330274 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(); + Python::with_gil(|py| { + // Uh oh, moved Rc to a new thread! + let c: &PyCell = obj.as_ref(py).downcast().unwrap(); - // 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 98d8adbf..5cd8a2d3 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 ad1b2c6d..def6f94c 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>`