Merge #2981

2981: Remove 0.17 deprecations r=adamreichold,davidhewitt a=davidhewitt

Since #2980 starts a breaking change for 0.19, let's also clean up all 0.17's deprecations.

I've removed `Python::acquire_gil` in its own commit, as that was a reasonably chunky removal.

Co-authored-by: Adam Reichold <adam.reichold@t-online.de>
Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>

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

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.

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.
 

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:

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<SomeClass>,
+    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<SomeClass>,
+}
+
+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

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

pytests/src/misc.rs

@@ -2,21 +2,12 @@ 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(|_| {});
 }
 
 #[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(())
 }

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(

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<PyObject>"
-)]
-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<F, R>(&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<T> 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,