diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 42f22204..a9ba960d 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -645,8 +645,7 @@ impl<'a> FnSpec<'a> { #( #holders )* let result = #call; let initializer: _pyo3::PyClassInitializer::<#cls> = result.convert(py)?; - let cell = initializer.create_cell_from_subtype(py, _slf)?; - ::std::result::Result::Ok(cell as *mut _pyo3::ffi::PyObject) + _pyo3::impl_::pymethods::tp_new_impl(py, initializer, _slf) } } } diff --git a/src/impl_/coroutine.rs b/src/impl_/coroutine.rs index 9be95ba3..1d311940 100644 --- a/src/impl_/coroutine.rs +++ b/src/impl_/coroutine.rs @@ -1,15 +1,15 @@ use std::{ future::Future, - mem, ops::{Deref, DerefMut}, }; use crate::{ coroutine::{cancel::ThrowCallback, Coroutine}, instance::Bound, + pycell::impl_::PyClassBorrowChecker, pyclass::boolean_struct::False, types::{PyAnyMethods, PyString}, - IntoPy, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult, Python, + IntoPy, Py, PyAny, PyClass, PyErr, PyObject, PyResult, Python, }; pub fn new_coroutine( @@ -32,17 +32,16 @@ where } fn get_ptr(obj: &Py) -> *mut T { - // SAFETY: Py can be casted as *const PyCell - unsafe { &*(obj.as_ptr() as *const PyCell) }.get_ptr() + obj.get_class_object().get_ptr() } pub struct RefGuard(Py); impl RefGuard { pub fn new(obj: &Bound<'_, PyAny>) -> PyResult { - let owned = obj.downcast::()?; - mem::forget(owned.try_borrow()?); - Ok(RefGuard(owned.clone().unbind())) + let bound = obj.downcast::()?; + bound.get_class_object().borrow_checker().try_borrow()?; + Ok(RefGuard(bound.clone().unbind())) } } @@ -57,9 +56,11 @@ impl Deref for RefGuard { impl Drop for RefGuard { fn drop(&mut self) { Python::with_gil(|gil| { - #[allow(deprecated)] - let self_ref = self.0.bind(gil); - self_ref.release_ref() + self.0 + .bind(gil) + .get_class_object() + .borrow_checker() + .release_borrow() }) } } @@ -68,9 +69,9 @@ pub struct RefMutGuard>(Py); impl> RefMutGuard { pub fn new(obj: &Bound<'_, PyAny>) -> PyResult { - let owned = obj.downcast::()?; - mem::forget(owned.try_borrow_mut()?); - Ok(RefMutGuard(owned.clone().unbind())) + let bound = obj.downcast::()?; + bound.get_class_object().borrow_checker().try_borrow_mut()?; + Ok(RefMutGuard(bound.clone().unbind())) } } @@ -92,9 +93,11 @@ impl> DerefMut for RefMutGuard { impl> Drop for RefMutGuard { fn drop(&mut self) { Python::with_gil(|gil| { - #[allow(deprecated)] - let self_ref = self.0.bind(gil); - self_ref.release_mut() + self.0 + .bind(gil) + .get_class_object() + .borrow_checker() + .release_borrow_mut() }) } } diff --git a/src/impl_/pycell.rs b/src/impl_/pycell.rs index 39811aeb..93514c7b 100644 --- a/src/impl_/pycell.rs +++ b/src/impl_/pycell.rs @@ -1,2 +1,4 @@ //! Externally-accessible implementation of pycell -pub use crate::pycell::impl_::{GetBorrowChecker, PyClassMutability}; +pub use crate::pycell::impl_::{ + GetBorrowChecker, PyClassMutability, PyClassObject, PyClassObjectBase, PyClassObjectLayout, +}; diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index e2204fab..1a144f73 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -2,14 +2,13 @@ use crate::{ exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError}, ffi, impl_::freelist::FreeList, - impl_::pycell::{GetBorrowChecker, PyClassMutability}, + impl_::pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout}, internal_tricks::extract_c_string, - pycell::PyCellLayout, pyclass_init::PyObjectInit, types::any::PyAnyMethods, types::PyBool, - Borrowed, Py, PyAny, PyCell, PyClass, PyErr, PyMethodDefType, PyNativeType, PyResult, - PyTypeInfo, Python, + Borrowed, Py, PyAny, PyClass, PyErr, PyMethodDefType, PyNativeType, PyResult, PyTypeInfo, + Python, }; use std::{ borrow::Cow, @@ -26,13 +25,13 @@ pub use lazy_type_object::LazyTypeObject; /// Gets the offset of the dictionary from the start of the object in bytes. #[inline] pub fn dict_offset() -> ffi::Py_ssize_t { - PyCell::::dict_offset() + PyClassObject::::dict_offset() } /// Gets the offset of the weakref list from the start of the object in bytes. #[inline] pub fn weaklist_offset() -> ffi::Py_ssize_t { - PyCell::::weaklist_offset() + PyClassObject::::weaklist_offset() } /// Represents the `__dict__` field for `#[pyclass]`. @@ -883,6 +882,8 @@ macro_rules! generate_pyclass_richcompare_slot { } pub use generate_pyclass_richcompare_slot; +use super::pycell::PyClassObject; + /// Implements a freelist. /// /// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]` @@ -1095,7 +1096,7 @@ impl PyClassThreadChecker for ThreadCheckerImpl { /// Trait denoting that this class is suitable to be used as a base type for PyClass. pub trait PyClassBaseType: Sized { - type LayoutAsBase: PyCellLayout; + type LayoutAsBase: PyClassObjectLayout; type BaseNativeType; type Initializer: PyObjectInit; type PyClassMutability: PyClassMutability; @@ -1105,7 +1106,7 @@ pub trait PyClassBaseType: Sized { /// /// In the future this will be extended to immutable PyClasses too. impl PyClassBaseType for T { - type LayoutAsBase = crate::pycell::PyCell; + type LayoutAsBase = crate::impl_::pycell::PyClassObject; type BaseNativeType = T::BaseNativeType; type Initializer = crate::pyclass_init::PyClassInitializer; type PyClassMutability = T::PyClassMutability; @@ -1113,7 +1114,7 @@ impl PyClassBaseType for T { /// Implementation of tp_dealloc for pyclasses without gc pub(crate) unsafe extern "C" fn tp_dealloc(obj: *mut ffi::PyObject) { - crate::impl_::trampoline::dealloc(obj, PyCell::::tp_dealloc) + crate::impl_::trampoline::dealloc(obj, PyClassObject::::tp_dealloc) } /// Implementation of tp_dealloc for pyclasses with gc @@ -1122,7 +1123,7 @@ pub(crate) unsafe extern "C" fn tp_dealloc_with_gc(obj: *mut ffi::Py { ffi::PyObject_GC_UnTrack(obj.cast()); } - crate::impl_::trampoline::dealloc(obj, PyCell::::tp_dealloc) + crate::impl_::trampoline::dealloc(obj, PyClassObject::::tp_dealloc) } pub(crate) unsafe extern "C" fn get_sequence_item_from_mapping( diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 70c95ca0..bc950baf 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -7,8 +7,8 @@ use crate::pycell::{PyBorrowError, PyBorrowMutError}; use crate::pyclass::boolean_struct::False; use crate::types::{any::PyAnyMethods, PyModule, PyType}; use crate::{ - ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyRef, - PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, + ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyCell, PyClass, PyClassInitializer, PyErr, + PyObject, PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, }; use std::borrow::Cow; use std::ffi::CStr; @@ -569,3 +569,13 @@ impl<'py, T> std::ops::Deref for BoundRef<'_, 'py, T> { self.0 } } + +pub unsafe fn tp_new_impl( + py: Python<'_>, + initializer: PyClassInitializer, + target_type: *mut ffi::PyTypeObject, +) -> PyResult<*mut ffi::PyObject> { + initializer + .create_class_object_of_type(py, target_type) + .map(Bound::into_ptr) +} diff --git a/src/instance.rs b/src/instance.rs index bc655caa..66c530d5 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1,5 +1,5 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; -use crate::ffi_ptr_ext::FfiPtrExt; +use crate::impl_::pycell::PyClassObject; use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; use crate::pyclass::boolean_struct::{False, True}; use crate::type_object::HasPyGilRef; @@ -92,14 +92,7 @@ where py: Python<'py>, value: impl Into>, ) -> PyResult> { - let initializer = value.into(); - let obj = initializer.create_cell(py)?; - let ob = unsafe { - obj.cast::() - .assume_owned(py) - .downcast_into_unchecked() - }; - Ok(ob) + value.into().create_class_object(py) } } @@ -332,19 +325,11 @@ where where T: PyClass + Sync, { - let cell = self.get_cell(); - // SAFETY: The class itself is frozen and `Sync` and we do not access anything but `cell.contents.value`. - unsafe { &*cell.get_ptr() } + self.1.get() } - pub(crate) fn get_cell(&'py self) -> &'py PyCell { - let cell = self.as_ptr().cast::>(); - // SAFETY: Bound is known to contain an object which is laid out in memory as a - // PyCell. - // - // Strictly speaking for now `&'py PyCell` is part of the "GIL Ref" API, so this - // could use some further refactoring later to avoid going through this reference. - unsafe { &*cell } + pub(crate) fn get_class_object(&self) -> &PyClassObject { + self.1.get_class_object() } } @@ -887,10 +872,7 @@ where /// # } /// ``` pub fn new(py: Python<'_>, value: impl Into>) -> PyResult> { - let initializer = value.into(); - let obj = initializer.create_cell(py)?; - let ob = unsafe { Py::from_owned_ptr(py, obj as _) }; - Ok(ob) + Bound::new(py, value).map(Bound::unbind) } } @@ -1194,12 +1176,16 @@ where where T: PyClass + Sync, { - let any = self.as_ptr() as *const PyAny; - // SAFETY: The class itself is frozen and `Sync` and we do not access anything but `cell.contents.value`. - unsafe { - let cell: &PyCell = PyNativeType::unchecked_downcast(&*any); - &*cell.get_ptr() - } + // Safety: The class itself is frozen and `Sync` + unsafe { &*self.get_class_object().get_ptr() } + } + + /// Get a view on the underlying `PyClass` contents. + pub(crate) fn get_class_object(&self) -> &PyClassObject { + let class_object = self.as_ptr().cast::>(); + // Safety: Bound is known to contain an object which is laid out in memory as a + // PyClassObject. + unsafe { &*class_object } } } diff --git a/src/pycell.rs b/src/pycell.rs index 9d00de95..43c75314 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -192,13 +192,10 @@ //! [guide]: https://pyo3.rs/latest/class.html#pycell-and-interior-mutability "PyCell and interior mutability" //! [Interior Mutability]: https://doc.rust-lang.org/book/ch15-05-interior-mutability.html "RefCell and the Interior Mutability Pattern - The Rust Programming Language" -#[allow(deprecated)] -use crate::conversion::FromPyPointer; +use crate::conversion::{AsPyPointer, ToPyObject}; use crate::exceptions::PyRuntimeError; use crate::ffi_ptr_ext::FfiPtrExt; -use crate::impl_::pyclass::{ - PyClassBaseType, PyClassDict, PyClassImpl, PyClassThreadChecker, PyClassWeakRef, -}; +use crate::impl_::pyclass::PyClassImpl; use crate::pyclass::{ boolean_struct::{False, True}, PyClass, @@ -207,28 +204,15 @@ use crate::pyclass_init::PyClassInitializer; use crate::type_object::{PyLayout, PySizedLayout}; use crate::types::any::PyAnyMethods; use crate::types::PyAny; -use crate::{ - conversion::{AsPyPointer, ToPyObject}, - type_object::get_tp_free, - PyTypeInfo, -}; use crate::{ffi, Bound, IntoPy, PyErr, PyNativeType, PyObject, PyResult, PyTypeCheck, Python}; -use std::cell::UnsafeCell; use std::fmt; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; pub(crate) mod impl_; -use impl_::{GetBorrowChecker, PyClassBorrowChecker, PyClassMutability}; +use impl_::PyClassBorrowChecker; -/// Base layout of PyCell. -#[doc(hidden)] -#[repr(C)] -pub struct PyCellBase { - ob_base: T, -} - -unsafe impl PyLayout for PyCellBase where U: PySizedLayout {} +use self::impl_::{PyClassObject, PyClassObjectLayout}; /// A container type for (mutably) accessing [`PyClass`] values /// @@ -266,20 +250,8 @@ unsafe impl PyLayout for PyCellBase where U: PySizedLayout {} /// ``` /// For more information on how, when and why (not) to use `PyCell` please see the /// [module-level documentation](self). -#[repr(C)] -pub struct PyCell { - ob_base: ::LayoutAsBase, - contents: PyCellContents, -} - -#[repr(C)] -pub(crate) struct PyCellContents { - pub(crate) value: ManuallyDrop>, - pub(crate) borrow_checker: ::Storage, - pub(crate) thread_checker: T::ThreadChecker, - pub(crate) dict: T::Dict, - pub(crate) weakref: T::WeakRef, -} +#[repr(transparent)] +pub struct PyCell(PyClassObject); unsafe impl PyNativeType for PyCell { type AsRefSource = T; @@ -298,12 +270,7 @@ impl PyCell { ) )] pub fn new(py: Python<'_>, value: impl Into>) -> PyResult<&Self> { - unsafe { - let initializer = value.into(); - let self_ = initializer.create_cell(py)?; - #[allow(deprecated)] - FromPyPointer::from_owned_ptr_or_err(py, self_ as _) - } + Bound::new(py, value).map(Bound::into_gil_ref) } /// Immutably borrows the value `T`. This borrow lasts as long as the returned `PyRef` exists. @@ -423,10 +390,11 @@ impl PyCell { /// }); /// ``` pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, PyBorrowError> { - self.ensure_threadsafe(); - self.borrow_checker() + self.0.ensure_threadsafe(); + self.0 + .borrow_checker() .try_borrow_unguarded() - .map(|_: ()| &*self.contents.value.get()) + .map(|_: ()| &*self.0.get_ptr()) } /// Provide an immutable borrow of the value `T` without acquiring the GIL. @@ -506,45 +474,7 @@ impl PyCell { } pub(crate) fn get_ptr(&self) -> *mut T { - self.contents.value.get() - } - - /// Gets the offset of the dictionary from the start of the struct in bytes. - pub(crate) fn dict_offset() -> ffi::Py_ssize_t { - use memoffset::offset_of; - - let offset = offset_of!(PyCell, contents) + offset_of!(PyCellContents, dict); - - // Py_ssize_t may not be equal to isize on all platforms - #[allow(clippy::useless_conversion)] - offset.try_into().expect("offset should fit in Py_ssize_t") - } - - /// Gets the offset of the weakref list from the start of the struct in bytes. - pub(crate) fn weaklist_offset() -> ffi::Py_ssize_t { - use memoffset::offset_of; - - let offset = offset_of!(PyCell, contents) + offset_of!(PyCellContents, weakref); - - // Py_ssize_t may not be equal to isize on all platforms - #[allow(clippy::useless_conversion)] - offset.try_into().expect("offset should fit in Py_ssize_t") - } - - #[cfg(feature = "macros")] - pub(crate) fn release_ref(&self) { - self.borrow_checker().release_borrow(); - } - - #[cfg(feature = "macros")] - pub(crate) fn release_mut(&self) { - self.borrow_checker().release_borrow_mut(); - } -} - -impl PyCell { - fn borrow_checker(&self) -> &::Checker { - T::PyClassMutability::borrow_checker(self) + self.0.get_ptr() } } @@ -675,7 +605,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - unsafe { &*self.inner.get_cell().ob_base.get_ptr() } + unsafe { &*self.inner.get_class_object().ob_base.get_ptr() } } } @@ -709,7 +639,7 @@ impl<'py, T: PyClass> PyRef<'py, T> { } pub(crate) fn try_borrow(obj: &Bound<'py, T>) -> Result { - let cell = obj.get_cell(); + let cell = obj.get_class_object(); cell.ensure_threadsafe(); cell.borrow_checker() .try_borrow() @@ -717,7 +647,7 @@ impl<'py, T: PyClass> PyRef<'py, T> { } pub(crate) fn try_borrow_threadsafe(obj: &Bound<'py, T>) -> Result { - let cell = obj.get_cell(); + let cell = obj.get_class_object(); cell.check_threadsafe()?; cell.borrow_checker() .try_borrow() @@ -793,13 +723,16 @@ impl<'p, T: PyClass> Deref for PyRef<'p, T> { #[inline] fn deref(&self) -> &T { - unsafe { &*self.inner.get_cell().get_ptr() } + unsafe { &*self.inner.get_class_object().get_ptr() } } } impl<'p, T: PyClass> Drop for PyRef<'p, T> { fn drop(&mut self) { - self.inner.get_cell().borrow_checker().release_borrow() + self.inner + .get_class_object() + .borrow_checker() + .release_borrow() } } @@ -856,7 +789,7 @@ where U: PyClass, { fn as_ref(&self) -> &T::BaseType { - unsafe { &*self.inner.get_cell().ob_base.get_ptr() } + unsafe { &*self.inner.get_class_object().ob_base.get_ptr() } } } @@ -866,7 +799,7 @@ where U: PyClass, { fn as_mut(&mut self) -> &mut T::BaseType { - unsafe { &mut *self.inner.get_cell().ob_base.get_ptr() } + unsafe { &mut *self.inner.get_class_object().ob_base.get_ptr() } } } @@ -900,7 +833,7 @@ impl<'py, T: PyClass> PyRefMut<'py, T> { } pub(crate) fn try_borrow(obj: &Bound<'py, T>) -> Result { - let cell = obj.get_cell(); + let cell = obj.get_class_object(); cell.ensure_threadsafe(); cell.borrow_checker() .try_borrow_mut() @@ -934,20 +867,23 @@ impl<'p, T: PyClass> Deref for PyRefMut<'p, T> { #[inline] fn deref(&self) -> &T { - unsafe { &*self.inner.get_cell().get_ptr() } + unsafe { &*self.inner.get_class_object().get_ptr() } } } impl<'p, T: PyClass> DerefMut for PyRefMut<'p, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.inner.get_cell().get_ptr() } + unsafe { &mut *self.inner.get_class_object().get_ptr() } } } impl<'p, T: PyClass> Drop for PyRefMut<'p, T> { fn drop(&mut self) { - self.inner.get_cell().borrow_checker().release_borrow_mut() + self.inner + .get_class_object() + .borrow_checker() + .release_borrow_mut() } } @@ -1034,81 +970,6 @@ impl From for PyErr { } } -#[doc(hidden)] -pub trait PyCellLayout: PyLayout { - fn ensure_threadsafe(&self); - fn check_threadsafe(&self) -> Result<(), PyBorrowError>; - /// Implementation of tp_dealloc. - /// # Safety - /// - slf must be a valid pointer to an instance of a T or a subclass. - /// - slf must not be used after this call (as it will be freed). - unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject); -} - -impl PyCellLayout for PyCellBase -where - U: PySizedLayout, - T: PyTypeInfo, -{ - fn ensure_threadsafe(&self) {} - fn check_threadsafe(&self) -> Result<(), PyBorrowError> { - Ok(()) - } - unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) { - let type_obj = T::type_object_raw(py); - // For `#[pyclass]` types which inherit from PyAny, we can just call tp_free - if type_obj == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) { - return get_tp_free(ffi::Py_TYPE(slf))(slf as _); - } - - // More complex native types (e.g. `extends=PyDict`) require calling the base's dealloc. - #[cfg(not(Py_LIMITED_API))] - { - if let Some(dealloc) = (*type_obj).tp_dealloc { - // Before CPython 3.11 BaseException_dealloc would use Py_GC_UNTRACK which - // assumes the exception is currently GC tracked, so we have to re-track - // before calling the dealloc so that it can safely call Py_GC_UNTRACK. - #[cfg(not(any(Py_3_11, PyPy)))] - if ffi::PyType_FastSubclass(type_obj, ffi::Py_TPFLAGS_BASE_EXC_SUBCLASS) == 1 { - ffi::PyObject_GC_Track(slf.cast()); - } - dealloc(slf as _); - } else { - get_tp_free(ffi::Py_TYPE(slf))(slf as _); - } - } - - #[cfg(Py_LIMITED_API)] - unreachable!("subclassing native types is not possible with the `abi3` feature"); - } -} - -impl PyCellLayout for PyCell -where - ::LayoutAsBase: PyCellLayout, -{ - fn ensure_threadsafe(&self) { - self.contents.thread_checker.ensure(); - self.ob_base.ensure_threadsafe(); - } - fn check_threadsafe(&self) -> Result<(), PyBorrowError> { - if !self.contents.thread_checker.check() { - return Err(PyBorrowError { _private: () }); - } - self.ob_base.check_threadsafe() - } - unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) { - // Safety: Python only calls tp_dealloc when no references to the object remain. - let cell = &mut *(slf as *mut PyCell); - if cell.contents.thread_checker.can_drop(py) { - ManuallyDrop::drop(&mut cell.contents.value); - } - cell.contents.dict.clear_dict(py); - cell.contents.weakref.clear_weakrefs(slf, py); - ::LayoutAsBase::tp_dealloc(py, slf) - } -} - #[cfg(test)] #[cfg(feature = "macros")] mod tests { diff --git a/src/pycell/impl_.rs b/src/pycell/impl_.rs index 29ba7eda..378bec04 100644 --- a/src/pycell/impl_.rs +++ b/src/pycell/impl_.rs @@ -1,11 +1,15 @@ #![allow(missing_docs)] -//! Crate-private implementation of pycell +//! Crate-private implementation of PyClassObject -use std::cell::Cell; +use std::cell::{Cell, UnsafeCell}; use std::marker::PhantomData; +use std::mem::ManuallyDrop; -use crate::impl_::pyclass::{PyClassBaseType, PyClassImpl}; -use crate::PyCell; +use crate::impl_::pyclass::{ + PyClassBaseType, PyClassDict, PyClassImpl, PyClassThreadChecker, PyClassWeakRef, +}; +use crate::type_object::{get_tp_free, PyLayout, PySizedLayout}; +use crate::{ffi, PyClass, PyTypeInfo, Python}; use super::{PyBorrowError, PyBorrowMutError}; @@ -156,29 +160,170 @@ impl PyClassBorrowChecker for BorrowChecker { } pub trait GetBorrowChecker { - fn borrow_checker(cell: &PyCell) -> &::Checker; + fn borrow_checker( + class_object: &PyClassObject, + ) -> &::Checker; } impl> GetBorrowChecker for MutableClass { - fn borrow_checker(cell: &PyCell) -> &BorrowChecker { - &cell.contents.borrow_checker + fn borrow_checker(class_object: &PyClassObject) -> &BorrowChecker { + &class_object.contents.borrow_checker } } impl> GetBorrowChecker for ImmutableClass { - fn borrow_checker(cell: &PyCell) -> &EmptySlot { - &cell.contents.borrow_checker + fn borrow_checker(class_object: &PyClassObject) -> &EmptySlot { + &class_object.contents.borrow_checker } } impl, M: PyClassMutability> GetBorrowChecker for ExtendsMutableAncestor where - T::BaseType: PyClassImpl + PyClassBaseType>, + T::BaseType: PyClassImpl + PyClassBaseType>, ::PyClassMutability: PyClassMutability, { - fn borrow_checker(cell: &PyCell) -> &BorrowChecker { - <::PyClassMutability as GetBorrowChecker>::borrow_checker(&cell.ob_base) + fn borrow_checker(class_object: &PyClassObject) -> &BorrowChecker { + <::PyClassMutability as GetBorrowChecker>::borrow_checker(&class_object.ob_base) + } +} + +/// Base layout of PyClassObject. +#[doc(hidden)] +#[repr(C)] +pub struct PyClassObjectBase { + ob_base: T, +} + +unsafe impl PyLayout for PyClassObjectBase where U: PySizedLayout {} + +#[doc(hidden)] +pub trait PyClassObjectLayout: PyLayout { + fn ensure_threadsafe(&self); + fn check_threadsafe(&self) -> Result<(), PyBorrowError>; + /// Implementation of tp_dealloc. + /// # Safety + /// - slf must be a valid pointer to an instance of a T or a subclass. + /// - slf must not be used after this call (as it will be freed). + unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject); +} + +impl PyClassObjectLayout for PyClassObjectBase +where + U: PySizedLayout, + T: PyTypeInfo, +{ + fn ensure_threadsafe(&self) {} + fn check_threadsafe(&self) -> Result<(), PyBorrowError> { + Ok(()) + } + unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) { + let type_obj = T::type_object_raw(py); + // For `#[pyclass]` types which inherit from PyAny, we can just call tp_free + if type_obj == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) { + return get_tp_free(ffi::Py_TYPE(slf))(slf.cast()); + } + + // More complex native types (e.g. `extends=PyDict`) require calling the base's dealloc. + #[cfg(not(Py_LIMITED_API))] + { + if let Some(dealloc) = (*type_obj).tp_dealloc { + // Before CPython 3.11 BaseException_dealloc would use Py_GC_UNTRACK which + // assumes the exception is currently GC tracked, so we have to re-track + // before calling the dealloc so that it can safely call Py_GC_UNTRACK. + #[cfg(not(any(Py_3_11, PyPy)))] + if ffi::PyType_FastSubclass(type_obj, ffi::Py_TPFLAGS_BASE_EXC_SUBCLASS) == 1 { + ffi::PyObject_GC_Track(slf.cast()); + } + dealloc(slf); + } else { + get_tp_free(ffi::Py_TYPE(slf))(slf.cast()); + } + } + + #[cfg(Py_LIMITED_API)] + unreachable!("subclassing native types is not possible with the `abi3` feature"); + } +} + +/// The layout of a PyClass as a Python object +#[repr(C)] +pub struct PyClassObject { + pub(crate) ob_base: ::LayoutAsBase, + contents: PyClassObjectContents, +} + +#[repr(C)] +pub(crate) struct PyClassObjectContents { + pub(crate) value: ManuallyDrop>, + pub(crate) borrow_checker: ::Storage, + pub(crate) thread_checker: T::ThreadChecker, + pub(crate) dict: T::Dict, + pub(crate) weakref: T::WeakRef, +} + +impl PyClassObject { + pub(crate) fn get_ptr(&self) -> *mut T { + self.contents.value.get() + } + + /// Gets the offset of the dictionary from the start of the struct in bytes. + pub(crate) fn dict_offset() -> ffi::Py_ssize_t { + use memoffset::offset_of; + + let offset = + offset_of!(PyClassObject, contents) + offset_of!(PyClassObjectContents, dict); + + // Py_ssize_t may not be equal to isize on all platforms + #[allow(clippy::useless_conversion)] + offset.try_into().expect("offset should fit in Py_ssize_t") + } + + /// Gets the offset of the weakref list from the start of the struct in bytes. + pub(crate) fn weaklist_offset() -> ffi::Py_ssize_t { + use memoffset::offset_of; + + let offset = + offset_of!(PyClassObject, contents) + offset_of!(PyClassObjectContents, weakref); + + // Py_ssize_t may not be equal to isize on all platforms + #[allow(clippy::useless_conversion)] + offset.try_into().expect("offset should fit in Py_ssize_t") + } +} + +impl PyClassObject { + pub(crate) fn borrow_checker(&self) -> &::Checker { + T::PyClassMutability::borrow_checker(self) + } +} + +unsafe impl PyLayout for PyClassObject {} +impl PySizedLayout for PyClassObject {} + +impl PyClassObjectLayout for PyClassObject +where + ::LayoutAsBase: PyClassObjectLayout, +{ + fn ensure_threadsafe(&self) { + self.contents.thread_checker.ensure(); + self.ob_base.ensure_threadsafe(); + } + fn check_threadsafe(&self) -> Result<(), PyBorrowError> { + if !self.contents.thread_checker.check() { + return Err(PyBorrowError { _private: () }); + } + self.ob_base.check_threadsafe() + } + unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) { + // Safety: Python only calls tp_dealloc when no references to the object remain. + let class_object = &mut *(slf.cast::>()); + if class_object.contents.thread_checker.can_drop(py) { + ManuallyDrop::drop(&mut class_object.contents.value); + } + class_object.contents.dict.clear_dict(py); + class_object.contents.weakref.clear_weakrefs(slf, py); + ::LayoutAsBase::tp_dealloc(py, slf) } } @@ -189,7 +334,6 @@ mod tests { use crate::prelude::*; use crate::pyclass::boolean_struct::{False, True}; - use crate::PyClass; #[pyclass(crate = "crate", subclass)] struct MutableBase; diff --git a/src/pyclass.rs b/src/pyclass.rs index ebdc52dc..eb4a5595 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -1,7 +1,7 @@ //! `PyClass` and related traits. use crate::{ - callback::IntoPyCallbackOutput, ffi, impl_::pyclass::PyClassImpl, Bound, IntoPy, PyCell, - PyObject, PyResult, PyTypeInfo, Python, + callback::IntoPyCallbackOutput, ffi, impl_::pyclass::PyClassImpl, IntoPy, PyCell, PyObject, + PyResult, PyTypeInfo, Python, }; use std::{cmp::Ordering, os::raw::c_int}; @@ -216,18 +216,6 @@ pub trait Frozen: boolean_struct::private::Boolean {} impl Frozen for boolean_struct::True {} impl Frozen for boolean_struct::False {} -impl<'py, T: PyClass> Bound<'py, T> { - #[cfg(feature = "macros")] - pub(crate) fn release_ref(&self) { - self.get_cell().release_ref(); - } - - #[cfg(feature = "macros")] - pub(crate) fn release_mut(&self) { - self.get_cell().release_mut(); - } -} - mod tests { #[test] fn test_compare_op_matches() { diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs index d0c271cf..52e34621 100644 --- a/src/pyclass/create_type_object.rs +++ b/src/pyclass/create_type_object.rs @@ -3,6 +3,7 @@ use pyo3_ffi::PyType_IS_GC; use crate::{ exceptions::PyTypeError, ffi, + impl_::pycell::PyClassObject, impl_::pyclass::{ assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, tp_dealloc_with_gc, PyClassItemsIter, @@ -13,7 +14,7 @@ use crate::{ }, types::typeobject::PyTypeMethods, types::PyType, - Py, PyCell, PyClass, PyGetterDef, PyMethodDefType, PyResult, PySetterDef, PyTypeInfo, Python, + Py, PyClass, PyGetterDef, PyMethodDefType, PyResult, PySetterDef, PyTypeInfo, Python, }; use std::{ borrow::Cow, @@ -94,7 +95,7 @@ where T::items_iter(), T::NAME, T::MODULE, - std::mem::size_of::>(), + std::mem::size_of::>(), ) } } diff --git a/src/pyclass_init.rs b/src/pyclass_init.rs index 94d377a7..923bc5b7 100644 --- a/src/pyclass_init.rs +++ b/src/pyclass_init.rs @@ -1,13 +1,12 @@ //! Contains initialization utilities for `#[pyclass]`. use crate::callback::IntoPyCallbackOutput; +use crate::ffi_ptr_ext::FfiPtrExt; use crate::impl_::pyclass::{PyClassBaseType, PyClassDict, PyClassThreadChecker, PyClassWeakRef}; -use crate::{ffi, Py, PyCell, PyClass, PyErr, PyResult, Python}; +use crate::types::PyAnyMethods; +use crate::{ffi, Bound, Py, PyClass, PyErr, PyResult, Python}; use crate::{ ffi::PyTypeObject, - pycell::{ - impl_::{PyClassBorrowChecker, PyClassMutability}, - PyCellContents, - }, + pycell::impl_::{PyClassBorrowChecker, PyClassMutability, PyClassObjectContents}, type_object::{get_tp_alloc, PyTypeInfo}, }; use std::{ @@ -207,29 +206,55 @@ impl PyClassInitializer { } /// Creates a new PyCell and initializes it. - #[doc(hidden)] - pub fn create_cell(self, py: Python<'_>) -> PyResult<*mut PyCell> + pub(crate) fn create_class_object(self, py: Python<'_>) -> PyResult> where T: PyClass, { - unsafe { self.create_cell_from_subtype(py, T::type_object_raw(py)) } + unsafe { self.create_class_object_of_type(py, T::type_object_raw(py)) } } - /// Creates a new PyCell and initializes it given a typeobject `subtype`. - /// Called by the Python `tp_new` implementation generated by a `#[new]` function in a `#[pymethods]` block. + /// Creates a new class object and initializes it given a typeobject `subtype`. /// /// # Safety /// `subtype` must be a valid pointer to the type object of T or a subclass. - #[doc(hidden)] - pub unsafe fn create_cell_from_subtype( + pub(crate) unsafe fn create_class_object_of_type( self, py: Python<'_>, - subtype: *mut crate::ffi::PyTypeObject, - ) -> PyResult<*mut PyCell> + target_type: *mut crate::ffi::PyTypeObject, + ) -> PyResult> where T: PyClass, { - self.into_new_object(py, subtype).map(|obj| obj as _) + /// Layout of a PyClassObject after base new has been called, but the contents have not yet been + /// written. + #[repr(C)] + struct PartiallyInitializedClassObject { + _ob_base: ::LayoutAsBase, + contents: MaybeUninit>, + } + + let (init, super_init) = match self.0 { + PyClassInitializerImpl::Existing(value) => return Ok(value.into_bound(py)), + PyClassInitializerImpl::New { init, super_init } => (init, super_init), + }; + + let obj = super_init.into_new_object(py, target_type)?; + + let part_init: *mut PartiallyInitializedClassObject = obj.cast(); + std::ptr::write( + (*part_init).contents.as_mut_ptr(), + PyClassObjectContents { + value: ManuallyDrop::new(UnsafeCell::new(init)), + borrow_checker: ::Storage::new(), + thread_checker: T::ThreadChecker::new(), + dict: T::Dict::INIT, + weakref: T::WeakRef::INIT, + }, + ); + + // Safety: obj is a valid pointer to an object of type `target_type`, which` is a known + // subclass of `T` + Ok(obj.assume_owned(py).downcast_into_unchecked()) } } @@ -239,33 +264,8 @@ impl PyObjectInit for PyClassInitializer { py: Python<'_>, subtype: *mut PyTypeObject, ) -> PyResult<*mut ffi::PyObject> { - /// Layout of a PyCell after base new has been called, but the contents have not yet been - /// written. - #[repr(C)] - struct PartiallyInitializedPyCell { - _ob_base: ::LayoutAsBase, - contents: MaybeUninit>, - } - - let (init, super_init) = match self.0 { - PyClassInitializerImpl::Existing(value) => return Ok(value.into_ptr()), - PyClassInitializerImpl::New { init, super_init } => (init, super_init), - }; - - let obj = super_init.into_new_object(py, subtype)?; - - let cell: *mut PartiallyInitializedPyCell = obj as _; - std::ptr::write( - (*cell).contents.as_mut_ptr(), - PyCellContents { - value: ManuallyDrop::new(UnsafeCell::new(init)), - borrow_checker: ::Storage::new(), - thread_checker: T::ThreadChecker::new(), - dict: T::Dict::INIT, - weakref: T::WeakRef::INIT, - }, - ); - Ok(obj) + self.create_class_object_of_type(py, subtype) + .map(Bound::into_ptr) } private_impl! {} diff --git a/src/type_object.rs b/src/type_object.rs index 318810b5..84888bee 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -6,7 +6,7 @@ use crate::types::{PyAny, PyType}; use crate::{ffi, Bound, PyNativeType, Python}; /// `T: PyLayout` represents that `T` is a concrete representation of `U` in the Python heap. -/// E.g., `PyCell` is a concrete representation of all `pyclass`es, and `ffi::PyObject` +/// E.g., `PyClassObject` is a concrete representation of all `pyclass`es, and `ffi::PyObject` /// is of `PyAny`. /// /// This trait is intended to be used internally. diff --git a/src/types/mod.rs b/src/types/mod.rs index 938716f7..66312a98 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -290,7 +290,7 @@ macro_rules! pyobject_native_type_sized { unsafe impl $crate::type_object::PyLayout<$name> for $layout {} impl $crate::type_object::PySizedLayout<$name> for $layout {} impl<$($generics,)*> $crate::impl_::pyclass::PyClassBaseType for $name { - type LayoutAsBase = $crate::pycell::PyCellBase<$layout>; + type LayoutAsBase = $crate::impl_::pycell::PyClassObjectBase<$layout>; type BaseNativeType = $name; type Initializer = $crate::pyclass_init::PyNativeTypeInitializer; type PyClassMutability = $crate::pycell::impl_::ImmutableClass; diff --git a/tests/test_coroutine.rs b/tests/test_coroutine.rs index e04aafda..db79c72a 100644 --- a/tests/test_coroutine.rs +++ b/tests/test_coroutine.rs @@ -3,6 +3,7 @@ use std::{task::Poll, thread, time::Duration}; use futures::{channel::oneshot, future::poll_fn, FutureExt}; +use portable_atomic::{AtomicBool, Ordering}; use pyo3::{ coroutine::CancelHandle, prelude::*, @@ -259,6 +260,15 @@ fn test_async_method_receiver() { self.0 } } + + static IS_DROPPED: AtomicBool = AtomicBool::new(false); + + impl Drop for Counter { + fn drop(&mut self) { + IS_DROPPED.store(true, Ordering::SeqCst); + } + } + Python::with_gil(|gil| { let test = r#" import asyncio @@ -291,5 +301,7 @@ fn test_async_method_receiver() { "#; let locals = [("Counter", gil.get_type_bound::())].into_py_dict_bound(gil); py_run!(gil, *locals, test); - }) + }); + + assert!(IS_DROPPED.load(Ordering::SeqCst)); }