diff --git a/src/ffi_ptr_ext.rs b/src/ffi_ptr_ext.rs index 183b0e3734e..956b4e8406c 100644 --- a/src/ffi_ptr_ext.rs +++ b/src/ffi_ptr_ext.rs @@ -6,10 +6,20 @@ use crate::{ }; pub(crate) trait FfiPtrExt: Sealed { + /// Assumes this pointer carries a Python reference which needs to be decref'd. + /// + /// If the pointer is NULL, this function will fetch an error. unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult>; + + /// Same as `assume_owned_or_err`, but doesn't fetch an error on NULL. unsafe fn assume_owned_or_opt(self, py: Python<'_>) -> Option>; + + /// Same as `assume_owned_or_err`, but panics on NULL. unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny>; + /// Same as `assume_owned_or_err`, but does not check for NULL. + unsafe fn assume_owned_unchecked(self, py: Python<'_>) -> Bound<'_, PyAny>; + /// Assumes this pointer is borrowed from a parent object. /// /// Warning: the lifetime `'a` is not bounded by the function arguments; the caller is @@ -44,6 +54,11 @@ impl FfiPtrExt for *mut ffi::PyObject { Bound::from_owned_ptr(py, self) } + #[inline] + unsafe fn assume_owned_unchecked(self, py: Python<'_>) -> Bound<'_, PyAny> { + Bound::from_owned_ptr_unchecked(py, self) + } + #[inline] unsafe fn assume_borrowed_or_err<'a>( self, diff --git a/src/instance.rs b/src/instance.rs index 1e15624929e..4d5608a9dc0 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -134,6 +134,19 @@ impl<'py> Bound<'py, PyAny> { Py::from_owned_ptr_or_err(py, ptr).map(|obj| Self(py, ManuallyDrop::new(obj))) } + /// Constructs a new `Bound<'py, PyAny>` from a pointer without checking for null. + /// + /// # Safety + /// + /// - `ptr` must be a valid pointer to a Python object + /// - `ptr` must be a strong/owned reference + pub(crate) unsafe fn from_owned_ptr_unchecked( + py: Python<'py>, + ptr: *mut ffi::PyObject, + ) -> Self { + Self(py, ManuallyDrop::new(Py::from_owned_ptr_unchecked(ptr))) + } + /// Constructs a new `Bound<'py, PyAny>` from a pointer by creating a new Python reference. /// Panics if `ptr` is null. /// @@ -1630,6 +1643,15 @@ impl Py { NonNull::new(ptr).map(|nonnull_ptr| Py(nonnull_ptr, PhantomData)) } + /// Constructs a new `Py` instance by taking ownership of the given FFI pointer. + /// + /// # Safety + /// + /// - `ptr` must be a non-null pointer to a Python object or type `T`. + pub(crate) unsafe fn from_owned_ptr_unchecked(ptr: *mut ffi::PyObject) -> Self { + Py(NonNull::new_unchecked(ptr), PhantomData) + } + /// Create a `Py` instance by creating a new reference from the given FFI pointer. /// /// # Safety diff --git a/src/pycell.rs b/src/pycell.rs index 438f7245a22..38f82eb27fd 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -370,7 +370,7 @@ where inner: unsafe { ManuallyDrop::new(self) .as_ptr() - .assume_owned(py) + .assume_owned_unchecked(py) .downcast_into_unchecked() }, } @@ -587,7 +587,7 @@ where inner: unsafe { ManuallyDrop::new(self) .as_ptr() - .assume_owned(py) + .assume_owned_unchecked(py) .downcast_into_unchecked() }, } diff --git a/src/types/dict.rs b/src/types/dict.rs index c4de7bae46a..61767f245cc 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -261,7 +261,11 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> { } { std::os::raw::c_int::MIN..=-1 => Err(PyErr::fetch(py)), 0 => Ok(None), - 1..=std::os::raw::c_int::MAX => Ok(Some(unsafe { result.assume_owned(py) })), + 1..=std::os::raw::c_int::MAX => { + // Safety: PyDict_GetItemRef positive return value means the result is a valid + // owned reference + Ok(Some(unsafe { result.assume_owned_unchecked(py) })) + } } } diff --git a/src/types/weakref/anyref.rs b/src/types/weakref/anyref.rs index 06d91ea024e..a3632544684 100644 --- a/src/types/weakref/anyref.rs +++ b/src/types/weakref/anyref.rs @@ -395,7 +395,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakref> { match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)"), 0 => PyNone::get(self.py()).to_owned().into_any(), - 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + // Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is + // a valid strong reference. + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) }, } } } diff --git a/src/types/weakref/proxy.rs b/src/types/weakref/proxy.rs index 798b4b435c7..a16c7583882 100644 --- a/src/types/weakref/proxy.rs +++ b/src/types/weakref/proxy.rs @@ -187,7 +187,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefProxy> { match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)"), 0 => PyNone::get(self.py()).to_owned().into_any(), - 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + // Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is + // a valid strong reference. + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) }, } } } diff --git a/src/types/weakref/reference.rs b/src/types/weakref/reference.rs index cc8bc3d55f5..40890e0f887 100644 --- a/src/types/weakref/reference.rs +++ b/src/types/weakref/reference.rs @@ -196,7 +196,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefReference> { match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } { std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)"), 0 => PyNone::get(self.py()).to_owned().into_any(), - 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) }, + // Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is + // a valid strong reference. + 1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) }, } } }