diff --git a/newsfragments/4410.added.md b/newsfragments/4410.added.md new file mode 100644 index 00000000000..350a54531bd --- /dev/null +++ b/newsfragments/4410.added.md @@ -0,0 +1 @@ +Add ffi binding `PyList_GetItemRef` and `pyo3_ffi::compat::PyList_GetItemRef` diff --git a/newsfragments/4410.fixed.md b/newsfragments/4410.fixed.md new file mode 100644 index 00000000000..f4403409aea --- /dev/null +++ b/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. diff --git a/pyo3-ffi/src/compat.rs b/pyo3-ffi/src/compat.rs index ef9a3fbe1c9..85986817d93 100644 --- a/pyo3-ffi/src/compat.rs +++ b/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 +} diff --git a/pyo3-ffi/src/listobject.rs b/pyo3-ffi/src/listobject.rs index a6f47eadf83..1096f2fe0c8 100644 --- a/pyo3-ffi/src/listobject.rs +++ b/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")] diff --git a/src/types/list.rs b/src/types/list.rs index 9cfd574cd0f..5dbfa0daa41 100644 --- a/src/types/list.rs +++ b/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> { 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()) } }