diff --git a/guide/src/class.md b/guide/src/class.md index 95c9f1877c8..b496365884c 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -564,6 +564,11 @@ impl MyClass { } ``` +Note that defining a class attribute of the same type as the class will make the class unusable +attempting to use the class will cause a panic reading `Recursive evaluation of type_object`. +If having the attribute on instances is acceptable, create a `#[getter]`, which can also use a +`OnceCell` for caching. + ## Callable objects To specify a custom `__call__` method for a custom class, the method needs to be annotated with @@ -921,10 +926,10 @@ unsafe impl pyo3::PyTypeInfo for MyClass { const FLAGS: usize = 0; #[inline] - fn type_object() -> &'static pyo3::ffi::PyTypeObject { + fn type_object_raw(py: pyo3::Python) -> &'static pyo3::ffi::PyTypeObject { use pyo3::type_object::LazyStaticType; static TYPE_OBJECT: LazyStaticType = LazyStaticType::new(); - TYPE_OBJECT.get_or_init::() + TYPE_OBJECT.get_or_init::(py) } } diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 33a45ba61c7..19930c51517 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -387,10 +387,10 @@ fn impl_class( const FLAGS: usize = #(#flags)|* | #extended; #[inline] - fn type_object() -> &'static pyo3::ffi::PyTypeObject { + fn type_object_raw(py: pyo3::Python) -> &'static pyo3::ffi::PyTypeObject { use pyo3::type_object::LazyStaticType; static TYPE_OBJECT: LazyStaticType = LazyStaticType::new(); - TYPE_OBJECT.get_or_init::() + TYPE_OBJECT.get_or_init::(py) } } diff --git a/src/freelist.rs b/src/freelist.rs index d3a4ae4e413..f7a6e4c6ed1 100644 --- a/src/freelist.rs +++ b/src/freelist.rs @@ -72,12 +72,12 @@ impl PyClassAlloc for T where T: PyTypeInfo + PyClassWithFreeList, { - unsafe fn alloc(_py: Python) -> *mut Self::Layout { + unsafe fn alloc(py: Python) -> *mut Self::Layout { if let Some(obj) = ::get_free_list().pop() { - ffi::PyObject_Init(obj, ::type_object() as *const _ as _); + ffi::PyObject_Init(obj, Self::type_object_raw(py) as *const _ as _); obj as _ } else { - crate::pyclass::default_alloc::() as _ + crate::pyclass::default_alloc::(py) as _ } } @@ -90,7 +90,7 @@ where } if let Some(obj) = ::get_free_list().insert(obj) { - match Self::type_object().tp_free { + match Self::type_object_raw(py).tp_free { Some(free) => free(obj as *mut c_void), None => tp_free_fallback(obj), } diff --git a/src/pyclass.rs b/src/pyclass.rs index 9d7bc8def10..06f2c59e07d 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -10,11 +10,11 @@ use std::os::raw::c_void; use std::ptr; #[inline] -pub(crate) unsafe fn default_alloc() -> *mut ffi::PyObject { - let type_obj = T::type_object(); +pub(crate) unsafe fn default_alloc(py: Python) -> *mut ffi::PyObject { + let type_obj = T::type_object_raw(py); // if the class derives native types(e.g., PyDict), call special new if T::FLAGS & type_flags::EXTENDED != 0 && T::BaseLayout::IS_NATIVE_TYPE { - let base_tp = ::type_object(); + let base_tp = T::BaseType::type_object_raw(py); if let Some(base_new) = base_tp.tp_new { return base_new(type_obj as *const _ as _, ptr::null_mut(), ptr::null_mut()); } @@ -29,8 +29,8 @@ pub trait PyClassAlloc: PyTypeInfo + Sized { /// /// # Safety /// This function must return a valid pointer to the Python heap. - unsafe fn alloc(_py: Python) -> *mut Self::Layout { - default_alloc::() as _ + unsafe fn alloc(py: Python) -> *mut Self::Layout { + default_alloc::(py) as _ } /// Deallocate `#[pyclass]` on the Python heap. @@ -44,7 +44,7 @@ pub trait PyClassAlloc: PyTypeInfo + Sized { return; } - match Self::type_object().tp_free { + match Self::type_object_raw(py).tp_free { Some(free) => free(obj as *mut c_void), None => tp_free_fallback(obj), } @@ -101,7 +101,7 @@ where s => CString::new(s)?.into_raw(), }; - type_object.tp_base = ::type_object() as *const _ as _; + type_object.tp_base = ::type_object_raw(py) as *const _ as _; type_object.tp_name = match module_name { Some(module_name) => CString::new(format!("{}.{}", module_name, T::NAME))?.into_raw(), diff --git a/src/type_object.rs b/src/type_object.rs index f530801059f..5f0b18968b2 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -1,12 +1,14 @@ // Copyright (c) 2017-present PyO3 Project and Contributors //! Python type object information +use crate::once_cell::OnceCell; use crate::pyclass::{initialize_type_object, PyClass}; use crate::pyclass_init::PyObjectInit; use crate::types::{PyAny, PyType}; -use crate::{ffi, AsPyPointer, Python}; -use std::cell::UnsafeCell; -use std::sync::atomic::{AtomicBool, Ordering}; +use crate::{ffi, AsPyPointer, Python, PyNativeType}; +use parking_lot::{Mutex, const_mutex}; +use std::collections::HashSet; +use std::thread::{self, ThreadId}; /// `T: PyLayout` represents that `T` is a concrete representaion of `U` in Python heap. /// E.g., `PyCell` is a concrete representaion of all `pyclass`es, and `ffi::PyObject` @@ -100,18 +102,18 @@ pub unsafe trait PyTypeInfo: Sized { type AsRefTarget: crate::PyNativeType; /// PyTypeObject instance for this type. - fn type_object() -> &'static ffi::PyTypeObject; + fn type_object_raw(py: Python) -> &'static ffi::PyTypeObject; /// Check if `*mut ffi::PyObject` is instance of this type fn is_instance(object: &PyAny) -> bool { unsafe { - ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object() as *const _ as _) != 0 + ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object(object.py()) as *const _ as _) != 0 } } /// Check if `*mut ffi::PyObject` is exact instance of this type fn is_exact_instance(object: &PyAny) -> bool { - unsafe { (*object.as_ptr()).ob_type == Self::type_object() as *const _ as _ } + unsafe { (*object.as_ptr()).ob_type == Self::type_object(object.py()) as *const _ as _ } } } @@ -131,39 +133,60 @@ where T: PyTypeInfo, { fn type_object(py: Python) -> &PyType { - unsafe { py.from_borrowed_ptr(::type_object() as *const _ as _) } + unsafe { py.from_borrowed_ptr(::type_object_raw(py) as *const _ as _) } } } /// Lazy type object for PyClass #[doc(hidden)] pub struct LazyStaticType { - value: UnsafeCell, - initialized: AtomicBool, + // Boxed because Python expects the type object to have a stable address. + value: OnceCell>, + // Threads which have begun initialization. Used for reentrant initialization detection. + initializing_threads: Mutex>> } impl LazyStaticType { pub const fn new() -> Self { LazyStaticType { - value: UnsafeCell::new(ffi::PyTypeObject_INIT), - initialized: AtomicBool::new(false), + value: OnceCell::new(), + initializing_threads: const_mutex(None) } } - pub fn get_or_init(&self) -> &ffi::PyTypeObject { - if !self - .initialized - .compare_and_swap(false, true, Ordering::Acquire) - { - let gil = Python::acquire_gil(); - let py = gil.python(); - initialize_type_object::(py, T::MODULE, unsafe { &mut *self.value.get() }) + pub fn get_or_init(&self, py: Python) -> &ffi::PyTypeObject { + self.value.get_or_init(py, || { + { + // Code evaluated at class init time, such as class attributes, might lead to + // recursive initalization of the type object if the class attribute is the same + // type as the class. + // + // That could lead to all sorts of unsafety such as using incomplete type objects + // to initialize class instances, so recursive initialization is prevented. + let thread_not_already_initializing = self.initializing_threads.lock() + .get_or_insert_with(HashSet::new) + .insert(thread::current().id()); + + if !thread_not_already_initializing { + panic!("Recursive initialization of type_object for {}", T::NAME); + } + } + + // Okay, not recursive initialization - can proceed safely. + let mut type_object = Box::new(ffi::PyTypeObject_INIT); + + initialize_type_object::(py, T::MODULE, type_object.as_mut()) .unwrap_or_else(|e| { e.print(py); panic!("An error occurred while initializing class {}", T::NAME) }); - } - unsafe { &*self.value.get() } + + // Initialization successfully complete, can clear the thread list. + // (No futher calls to get_or_init() will try to init, on any thread.) + *self.initializing_threads.lock() = None; + + type_object + }).as_ref() } } diff --git a/src/types/mod.rs b/src/types/mod.rs index 66e53e5d2d0..6e921a2e6ba 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -143,7 +143,7 @@ macro_rules! pyobject_native_type_convert( const MODULE: Option<&'static str> = $module; #[inline] - fn type_object() -> &'static $crate::ffi::PyTypeObject { + fn type_object_raw(_py: Python) -> &'static $crate::ffi::PyTypeObject { unsafe{ &$typeobject } } diff --git a/tests/test_class_attributes.rs b/tests/test_class_attributes.rs index 085e2faf43a..17243556eb4 100644 --- a/tests/test_class_attributes.rs +++ b/tests/test_class_attributes.rs @@ -30,25 +30,12 @@ impl Foo { "bar".to_string() } - #[classattr] - fn foo() -> Foo { - Foo { x: 1 } - } - #[classattr] fn bar() -> Bar { Bar { x: 2 } } } -#[pymethods] -impl Bar { - #[classattr] - fn foo() -> Foo { - Foo { x: 3 } - } -} - #[test] fn class_attributes() { let gil = Python::acquire_gil(); @@ -67,13 +54,23 @@ fn class_attributes_are_immutable() { py_expect_exception!(py, foo_obj, "foo_obj.a = 6", TypeError); } +#[pyclass] +struct SelfClassAttribute { + #[pyo3(get)] + x: i32 +} + +#[pymethods] +impl SelfClassAttribute { + #[classattr] + const SELF: SelfClassAttribute = SelfClassAttribute { x: 1 }; +} + #[test] +#[should_panic(expected = "Recursive initialization of type_object for SelfClassAttribute")] fn recursive_class_attributes() { let gil = Python::acquire_gil(); let py = gil.python(); - let foo_obj = py.get_type::(); - let bar_obj = py.get_type::(); - py_assert!(py, foo_obj, "foo_obj.foo.x == 1"); - py_assert!(py, foo_obj, "foo_obj.bar.x == 2"); - py_assert!(py, bar_obj, "bar_obj.foo.x == 3"); + + py.get_type::(); }