Add PyList_GetItemRef and use it in list.get_item (#4410)

* Add PyList_GetItemRef bindings and compat shim

* Use PyList_GetItemRef in list.get_item()

* add release notes

* apply code review comments

* Update newsfragments/4410.added.md

Co-authored-by: David Hewitt <[email protected]>

* apply `all()` doc cfg hints

---------

Co-authored-by: David Hewitt <[email protected]>

Author: ngoldbaum

newsfragments/4410.added.md

@@ -0,0 +1 @@
+Add ffi binding `PyList_GetItemRef` and `pyo3_ffi::compat::PyList_GetItemRef`

newsfragments/4410.fixed.md

@@ -0,0 +1,3 @@
+* Avoid creating temporary borrowed reference in list.get_item
+  bindings. Temporary borrowed references are unsafe in the free-threaded python
+  build if the list is shared between threads.

pyo3-ffi/src/compat.rs

@@ -12,13 +12,15 @@
 #[cfg(not(Py_3_13))]
 use crate::object::PyObject;
 #[cfg(not(Py_3_13))]
+use crate::pyport::Py_ssize_t;
+#[cfg(not(Py_3_13))]
 use std::os::raw::c_int;
 
-#[cfg_attr(docsrs, doc(cfg(all)))]
+#[cfg_attr(docsrs, doc(cfg(all())))]
 #[cfg(Py_3_13)]
 pub use crate::dictobject::PyDict_GetItemRef;
 
-#[cfg_attr(docsrs, doc(cfg(all)))]
+#[cfg_attr(docsrs, doc(cfg(all())))]
 #[cfg(not(Py_3_13))]
 pub unsafe fn PyDict_GetItemRef(
     dp: *mut PyObject,
@@ -42,3 +44,17 @@ pub unsafe fn PyDict_GetItemRef(
         -1
     }
 }
+
+#[cfg_attr(docsrs, doc(cfg(all())))]
+#[cfg(Py_3_13)]
+pub use crate::PyList_GetItemRef;
+
+#[cfg(not(Py_3_13))]
+#[cfg_attr(docsrs, doc(cfg(all())))]
+pub unsafe fn PyList_GetItemRef(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject {
+    use crate::{PyList_GetItem, Py_XINCREF};
+
+    let item: *mut PyObject = PyList_GetItem(arg1, arg2);
+    Py_XINCREF(item);
+    item
+}

pyo3-ffi/src/listobject.rs

@@ -28,6 +28,8 @@ extern "C" {
     pub fn PyList_Size(arg1: *mut PyObject) -> Py_ssize_t;
     #[cfg_attr(PyPy, link_name = "PyPyList_GetItem")]
     pub fn PyList_GetItem(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject;
+    #[cfg(Py_3_13)]
+    pub fn PyList_GetItemRef(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject;
     #[cfg_attr(PyPy, link_name = "PyPyList_SetItem")]
     pub fn PyList_SetItem(arg1: *mut PyObject, arg2: Py_ssize_t, arg3: *mut PyObject) -> c_int;
     #[cfg_attr(PyPy, link_name = "PyPyList_Insert")]

src/types/list.rs

@@ -3,7 +3,6 @@ use std::iter::FusedIterator;
 use crate::err::{self, PyResult};
 use crate::ffi::{self, Py_ssize_t};
 use crate::ffi_ptr_ext::FfiPtrExt;
-use crate::instance::Borrowed;
 use crate::internal_tricks::get_ssize_index;
 use crate::types::{PySequence, PyTuple};
 #[cfg(feature = "gil-refs")]
@@ -434,10 +433,8 @@ impl<'py> PyListMethods<'py> for Bound<'py, PyList> {
     /// ```
     fn get_item(&self, index: usize) -> PyResult<Bound<'py, PyAny>> {
         unsafe {
-            // PyList_GetItem return borrowed ptr; must make owned for safety (see #890).
-            ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t)
-                .assume_borrowed_or_err(self.py())
-                .map(Borrowed::to_owned)
+            ffi::compat::PyList_GetItemRef(self.as_ptr(), index as Py_ssize_t)
+                .assume_owned_or_err(self.py())
         }
     }