Add pyo3::once_cell::GILOnceCell

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 - Add FFI definition `PyObject_AsFileDescriptor` [#938](https://github.com/PyO3/pyo3/pull/938)
 - Add `PyByteArray::data`, `PyByteArray::as_bytes`, and `PyByteArray::as_bytes_mut`. [#967](https://github.com/PyO3/pyo3/pull/967)
+- Add `GILOnceCell` to use in situations where `lazy_static` or `once_cell` can deadlock. [#975](https://github.com/PyO3/pyo3/pull/975)
 - Add `Py::borrow`, `Py::borrow_mut`, `Py::try_borrow`, and `Py::try_borrow_mut` for accessing `#[pyclass]` values. [#976](https://github.com/PyO3/pyo3/pull/976)
 
 ### Changed
@@ -19,12 +20,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Change signature of `PyTypeObject::type_object()` - now takes `Python` argument and returns `&PyType`. [#970](https://github.com/PyO3/pyo3/pull/970)
 - Change return type of `PyTuple::slice()` and `PyTuple::split_from()` from `Py<PyTuple>` to `&PyTuple`. [#970](https://github.com/PyO3/pyo3/pull/970)
 - Change return type of `PyTuple::as_slice` to `&[&PyAny]`. [#971](https://github.com/PyO3/pyo3/pull/971)
+- Rename `PyTypeInfo::type_object` to `type_object_raw`, and add `Python` argument. [#975](https://github.com/PyO3/pyo3/pull/975)
 - Update `num-complex` optional dependendency from `0.2` to `0.3`. [#977](https://github.com/PyO3/pyo3/pull/977)
 - Update `num-bigint` optional dependendency from `0.2` to `0.3`. [#978](https://github.com/PyO3/pyo3/pull/978)
 - `#[pyproto]` is re-implemented without specialization. [#961](https://github.com/PyO3/pyo3/pull/961)
 
 ### Removed
 - Remove `ManagedPyRef` (unused, and needs specialization) [#930](https://github.com/PyO3/pyo3/pull/930)
+- Disable `#[classattr]` where the class attribute is the same type as the class. (This may be re-enabled in the future; the previous implemenation was unsound.) [#975](https://github.com/PyO3/pyo3/pull/975)
 
 ### Fixed
 - Fix passing explicit `None` to `Option<T>` argument `#[pyfunction]` with a default value. [#936](https://github.com/PyO3/pyo3/pull/936)

guide/src/SUMMARY.md

@@ -12,6 +12,7 @@
 - [Advanced Topics](advanced.md)
 - [Building and Distribution](building_and_distribution.md)
 - [PyPy support](pypy.md)
+- [FAQ & Troubleshooting](faq.md)
 - [Appendix A: PyO3 and rust-cpython](rust_cpython.md)
 - [Appendix B: Migration Guide](migration.md)
 - [Appendix C: Trait bounds](trait_bounds.md)

guide/src/class.md

@@ -565,6 +565,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`.
+As an alternative, if having the attribute on instances is acceptable, create a `#[getter]` which
+uses a `GILOnceCell` to cache the attribute value. Or add the attribute to a module instead.
+
 ## Callable objects
 
 To specify a custom `__call__` method for a custom class, the method needs to be annotated with
@@ -922,10 +927,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::<Self>()
+        TYPE_OBJECT.get_or_init::<Self>(py)
     }
 }
 

guide/src/faq.md

@@ -0,0 +1,16 @@
+# Frequently Asked Questions / Troubleshooting
+
+## I'm experiencing deadlocks using PyO3 with lazy_static or once_cell!
+
+`lazy_static` and `once_cell::sync` both use locks to ensure that initialization is performed only by a single thread. Because the Python GIL is an additional lock this can lead to deadlocks in the following way:
+
+1. A thread (thread A) which has acquired the Python GIL starts initialization of a `lazy_static` value.
+2. The initialization code calls some Python API which temporarily releases the GIL e.g. `Python::import`.
+3. Another thread (thread B) acquires the Python GIL and attempts to access the same `lazy_static` value.
+4. Thread B is blocked, because it waits for `lazy_static`'s initialization to lock to release.
+5. Thread A is blocked, because it waits to re-aquire the GIL which thread B still holds.
+6. Deadlock.
+
+PyO3 provides a struct [`GILOnceCell`] which works equivalently to `OnceCell` but relies solely on the Python GIL for thread safety. This means it can be used in place of `lazy_static` or `once_cell` where you are experiencing the deadlock described above. See the documentation for [`GILOnceCell`] for an example how to use it.
+
+[`GILOnceCell`]: https://docs.rs/pyo3/latest/pyo3/once_cell/struct.GILOnceCell.html

pyo3-derive-backend/src/pyclass.rs

@@ -401,10 +401,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::<Self>()
+                TYPE_OBJECT.get_or_init::<Self>(py)
             }
         }
 

src/exceptions.rs

@@ -90,28 +90,27 @@ macro_rules! import_exception_type_object {
     ($module: expr, $name: ident) => {
         unsafe impl $crate::type_object::PyTypeObject for $name {
             fn type_object(py: $crate::Python) -> &$crate::types::PyType {
-                use $crate::type_object::LazyHeapType;
+                use $crate::once_cell::GILOnceCell;
-                static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();
+                use $crate::AsPyRef;
+                static TYPE_OBJECT: GILOnceCell<$crate::Py<$crate::types::PyType>> =
+                    GILOnceCell::new();
 
-                let ptr = TYPE_OBJECT.get_or_init(|py| {
+                TYPE_OBJECT
-                    let imp = py
+                    .get_or_init(py, || {
-                        .import(stringify!($module))
+                        let imp = py
-                        .expect(concat!("Can not import module: ", stringify!($module)));
+                            .import(stringify!($module))
-                    let cls = imp.get(stringify!($name)).expect(concat!(
+                            .expect(concat!("Can not import module: ", stringify!($module)));
-                        "Can not load exception class: {}.{}",
+                        let cls = imp.get(stringify!($name)).expect(concat!(
-                        stringify!($module),
+                            "Can not load exception class: {}.{}",
-                        ".",
+                            stringify!($module),
-                        stringify!($name)
+                            ".",
-                    ));
+                            stringify!($name)
+                        ));
 
-                    unsafe {
+                        cls.extract()
-                        std::ptr::NonNull::new_unchecked(
+                            .expect("Imported exception should be a type object")
-                            $crate::IntoPyPointer::into_ptr(cls) as *mut _
+                    })
-                        )
+                    .as_ref(py)
-                    }
-                });
-
-                unsafe { py.from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
             }
         }
     };
@@ -174,19 +173,25 @@ macro_rules! create_exception_type_object {
     ($module: ident, $name: ident, $base: ty) => {
         unsafe impl $crate::type_object::PyTypeObject for $name {
             fn type_object(py: $crate::Python) -> &$crate::types::PyType {
-                use $crate::type_object::LazyHeapType;
+                use $crate::once_cell::GILOnceCell;
-                static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();
+                use $crate::AsPyRef;
+                static TYPE_OBJECT: GILOnceCell<$crate::Py<$crate::types::PyType>> =
+                    GILOnceCell::new();
 
-                let ptr = TYPE_OBJECT.get_or_init(|py| {
+                TYPE_OBJECT
-                    $crate::PyErr::new_type(
+                    .get_or_init(py, || unsafe {
-                        py,
+                        $crate::Py::from_owned_ptr(
-                        concat!(stringify!($module), ".", stringify!($name)),
+                            py,
-                        Some(py.get_type::<$base>()),
+                            $crate::PyErr::new_type(
-                        None,
+                                py,
-                    )
+                                concat!(stringify!($module), ".", stringify!($name)),
-                });
+                                Some(py.get_type::<$base>()),
-
+                                None,
-                unsafe { py.from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
+                            )
+                            .as_ptr() as *mut $crate::ffi::PyObject,
+                        )
+                    })
+                    .as_ref(py)
             }
         }
     };

src/ffi/datetime.rs

@@ -14,10 +14,10 @@
 use crate::ffi::Py_hash_t;
 use crate::ffi::{PyObject, PyTypeObject};
 use crate::ffi::{PyObject_TypeCheck, Py_TYPE};
+use crate::once_cell::GILOnceCell;
+use crate::Python;
 use std::ops::Deref;
 use std::os::raw::{c_char, c_int, c_uchar};
-use std::ptr;
-use std::sync::Once;
 #[cfg(not(PyPy))]
 use {crate::ffi::PyCapsule_Import, std::ffi::CString};
 
@@ -196,29 +196,9 @@ pub struct PyDateTime_Delta {
     pub microseconds: c_int,
 }
 
-// C API Capsule
+// Python already shares this object between threads, so it's no more evil for us to do it too!
-// Note: This is "roll-your-own" lazy-static implementation is necessary because
+unsafe impl Sync for PyDateTime_CAPI {}
-// of the interaction between the call_once locks and the GIL. It turns out that
+static PY_DATETIME_API: GILOnceCell<&'static PyDateTime_CAPI> = GILOnceCell::new();
-// calling PyCapsule_Import releases and re-acquires the GIL during the import,
-// so if you have two threads attempting to use the PyDateTimeAPI singleton
-// under the GIL, it causes a deadlock; what happens is:
-//
-// Thread 1 acquires GIL
-// Thread 1 acquires the call_once lock
-// Thread 1 calls PyCapsule_Import, thus releasing the GIL
-// Thread 2 acquires the GIL
-// Thread 2 blocks waiting for the call_once lock
-// Thread 1 blocks waiting for the GIL
-//
-// However, Python's import mechanism acquires a module-specific lock around
-// each import call, so all call importing datetime will return the same
-// object, making the call_once lock superfluous. As such, we can weaken
-// the guarantees of the cache, such that PyDateTime_IMPORT can be called
-// until __PY_DATETIME_API_UNSAFE_CACHE is populated, which will happen exactly
-// one time. So long as PyDateTime_IMPORT has no side effects (it should not),
-// this will be at most a slight waste of resources.
-static PY_DATETIME_API_ONCE: Once = Once::new();
-static mut PY_DATETIME_API_UNSAFE_CACHE: *const PyDateTime_CAPI = ptr::null();
 
 #[derive(Debug)]
 pub struct PyDateTimeAPI {
@@ -233,13 +213,7 @@ impl Deref for PyDateTimeAPI {
     type Target = PyDateTime_CAPI;
 
     fn deref(&self) -> &'static PyDateTime_CAPI {
-        unsafe {
+        unsafe { PyDateTime_IMPORT() }
-            if !PY_DATETIME_API_UNSAFE_CACHE.is_null() {
-                &(*PY_DATETIME_API_UNSAFE_CACHE)
-            } else {
-                PyDateTime_IMPORT()
-            }
-        }
     }
 }
 
@@ -251,25 +225,27 @@ impl Deref for PyDateTimeAPI {
 /// Use this function only if you want to eagerly load the datetime module,
 /// such as if you do not want the first call to a datetime function to be
 /// slightly slower than subsequent calls.
+///
+/// # Safety
+/// The Python GIL must be held.
 pub unsafe fn PyDateTime_IMPORT() -> &'static PyDateTime_CAPI {
-    // PyPy expects the C-API to be initialized via PyDateTime_Import, so trying to use
+    let py = Python::assume_gil_acquired();
-    // `PyCapsule_Import` will behave unexpectedly in pypy.
+    PY_DATETIME_API.get_or_init(py, || {
-    #[cfg(PyPy)]
+        // PyPy expects the C-API to be initialized via PyDateTime_Import, so trying to use
-    let py_datetime_c_api = PyDateTime_Import();
+        // `PyCapsule_Import` will behave unexpectedly in pypy.
+        #[cfg(PyPy)]
+        let py_datetime_c_api = PyDateTime_Import();
 
-    #[cfg(not(PyPy))]
+        #[cfg(not(PyPy))]
-    let py_datetime_c_api = {
+        let py_datetime_c_api = {
-        // PyDateTime_CAPSULE_NAME is a macro in C
+            // PyDateTime_CAPSULE_NAME is a macro in C
-        let PyDateTime_CAPSULE_NAME = CString::new("datetime.datetime_CAPI").unwrap();
+            let PyDateTime_CAPSULE_NAME = CString::new("datetime.datetime_CAPI").unwrap();
 
-        PyCapsule_Import(PyDateTime_CAPSULE_NAME.as_ptr(), 1) as *const PyDateTime_CAPI
+            &*(PyCapsule_Import(PyDateTime_CAPSULE_NAME.as_ptr(), 1) as *const PyDateTime_CAPI)
-    };
+        };
 
-    PY_DATETIME_API_ONCE.call_once(move || {
+        py_datetime_c_api
-        PY_DATETIME_API_UNSAFE_CACHE = py_datetime_c_api;
+    })
-    });
-
-    &(*PY_DATETIME_API_UNSAFE_CACHE)
 }
 
 /// Type Check macros

src/freelist.rs

@@ -72,12 +72,12 @@ impl<T> 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) = <Self as PyClassWithFreeList>::get_free_list().pop() {
-            ffi::PyObject_Init(obj, <Self as PyTypeInfo>::type_object() as *const _ as _);
+            ffi::PyObject_Init(obj, Self::type_object_raw(py) as *const _ as _);
             obj as _
         } else {
-            crate::pyclass::default_alloc::<Self>() as _
+            crate::pyclass::default_alloc::<Self>(py) as _
         }
     }
 
@@ -90,7 +90,7 @@ where
         }
 
         if let Some(obj) = <Self as PyClassWithFreeList>::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),
             }

src/lib.rs

@@ -184,6 +184,7 @@ mod instance;
 mod internal_tricks;
 pub mod marshal;
 mod object;
+pub mod once_cell;
 pub mod panic;
 pub mod prelude;
 pub mod pycell;

src/once_cell.rs

@@ -0,0 +1,100 @@
+use crate::Python;
+use std::cell::UnsafeCell;
+
+/// A write-once cell similar to [`once_cell::OnceCell`](https://docs.rs/once_cell/1.4.0/once_cell/).
+///
+/// Unlike `once_cell::sync` which blocks threads to achieve thread safety, this implementation
+/// uses the Python GIL to mediate concurrent access. This helps in cases where `once_sync` or
+/// `lazy_static`'s synchronization strategy can lead to deadlocks when interacting with the Python
+/// GIL. For an example, see [the FAQ section](https://pyo3.rs/master/faq.html) of the guide.
+///
+/// # Example
+///
+/// The following example shows how to use `GILOnceCell` to share a reference to a Python list
+/// between threads:
+///
+/// ```
+/// use pyo3::prelude::*;
+/// use pyo3::types::PyList;
+/// use pyo3::once_cell::GILOnceCell;
+///
+/// static LIST_CELL: GILOnceCell<Py<PyList>> = GILOnceCell::new();
+///
+/// pub fn get_shared_list(py: Python) -> &PyList {
+///     LIST_CELL
+///         .get_or_init(py, || PyList::empty(py).into())
+///         .as_ref(py)
+/// }
+/// # let gil = Python::acquire_gil();
+/// # let py = gil.python();
+/// # assert_eq!(get_shared_list(py).len(), 0 );
+/// ```
+pub struct GILOnceCell<T>(UnsafeCell<Option<T>>);
+
+// T: Send is needed for Sync because the thread which drops the GILOnceCell can be different
+// to the thread which fills it.
+unsafe impl<T: Send + Sync> Sync for GILOnceCell<T> {}
+unsafe impl<T: Send> Send for GILOnceCell<T> {}
+
+impl<T> GILOnceCell<T> {
+    /// Create a `GILOnceCell` which does not yet contain a value.
+    pub const fn new() -> Self {
+        Self(UnsafeCell::new(None))
+    }
+
+    /// Get a reference to the contained value, or `None` if the cell has not yet been written.
+    pub fn get(&self, _py: Python) -> Option<&T> {
+        // Safe because if the cell has not yet been written, None is returned.
+        unsafe { &*self.0.get() }.as_ref()
+    }
+
+    /// Get a reference to the contained value, initializing it if needed using the provided
+    /// closure.
+    ///
+    /// Note that:
+    ///  1) reentrant initialization can cause a stack overflow.
+    ///  2) if f() temporarily releases the GIL (e.g. by calling `Python::import`) then it is
+    ///     possible (and well-defined) that a second thread may also call get_or_init and begin
+    ///     calling `f()`. Even when this happens `GILOnceCell` guarantees that only **one** write
+    ///     to the cell ever occurs - other threads will simply discard the value they compute and
+    ///     return the result of the first complete computation.
+    pub fn get_or_init<F>(&self, py: Python, f: F) -> &T
+    where
+        F: FnOnce() -> T,
+    {
+        let inner = unsafe { &*self.0.get() }.as_ref();
+        if let Some(value) = inner {
+            return value;
+        }
+
+        // Note that f() could temporarily release the GIL, so it's possible that another thread
+        // writes to this GILOnceCell before f() finishes. That's fine; we'll just have to discard
+        // the value computed here and accept a bit of wasted computation.
+        let value = f();
+        let _ = self.set(py, value);
+
+        self.get(py).unwrap()
+    }
+
+    /// Get the contents of the cell mutably. This is only possible if the reference to the cell is
+    /// unique.
+    pub fn get_mut(&mut self) -> Option<&mut T> {
+        // Safe because we have &mut self
+        unsafe { &mut *self.0.get() }.as_mut()
+    }
+
+    /// Set the value in the cell.
+    ///
+    /// If the cell has already been written, `Err(value)` will be returned containing the new
+    /// value which was not written.
+    pub fn set(&self, _py: Python, value: T) -> Result<(), T> {
+        // Safe because GIL is held, so no other thread can be writing to this cell concurrently.
+        let inner = unsafe { &mut *self.0.get() };
+        if inner.is_some() {
+            return Err(value);
+        }
+
+        *inner = Some(value);
+        Ok(())
+    }
+}

src/pyclass.rs

@@ -11,11 +11,11 @@ use std::os::raw::c_void;
 use std::ptr;
 
 #[inline]
-pub(crate) unsafe fn default_alloc<T: PyTypeInfo>() -> *mut ffi::PyObject {
+pub(crate) unsafe fn default_alloc<T: PyTypeInfo>(py: Python) -> *mut ffi::PyObject {
-    let type_obj = T::type_object();
+    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 = <T::BaseType as PyTypeInfo>::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());
         }
@@ -30,8 +30,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 {
+    unsafe fn alloc(py: Python) -> *mut Self::Layout {
-        default_alloc::<Self>() as _
+        default_alloc::<Self>(py) as _
     }
 
     /// Deallocate `#[pyclass]` on the Python heap.
@@ -45,7 +45,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),
         }
@@ -107,7 +107,7 @@ where
         s => CString::new(s)?.into_raw(),
     };
 
-    type_object.tp_base = <T::BaseType as PyTypeInfo>::type_object() as *const _ as _;
+    type_object.tp_base = T::BaseType::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(),

src/type_object.rs

@@ -1,13 +1,13 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 //! Python type object information
 
+use crate::once_cell::GILOnceCell;
 use crate::pyclass::{initialize_type_object, PyClass};
 use crate::pyclass_init::PyObjectInit;
 use crate::types::{PyAny, PyType};
-use crate::{ffi, AsPyPointer, Python};
+use crate::{ffi, AsPyPointer, PyNativeType, Python};
-use std::cell::UnsafeCell;
+use parking_lot::{const_mutex, Mutex};
-use std::ptr::NonNull;
+use std::thread::{self, ThreadId};
-use std::sync::atomic::{AtomicBool, Ordering};
 
 /// `T: PyLayout<U>` 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`
@@ -101,18 +101,21 @@ 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 _ }
     }
 }
 
@@ -132,78 +135,63 @@ where
     T: PyTypeInfo,
 {
     fn type_object(py: Python) -> &PyType {
-        unsafe { py.from_borrowed_ptr(<Self as PyTypeInfo>::type_object() as *const _ as _) }
+        unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as *const _ as _) }
     }
 }
 
-/// Lazy type object for Exceptions
-#[doc(hidden)]
-pub struct LazyHeapType {
-    value: UnsafeCell<Option<NonNull<ffi::PyTypeObject>>>,
-    initialized: AtomicBool,
-}
-
-impl LazyHeapType {
-    pub const fn new() -> Self {
-        LazyHeapType {
-            value: UnsafeCell::new(None),
-            initialized: AtomicBool::new(false),
-        }
-    }
-
-    pub fn get_or_init<F>(&self, constructor: F) -> NonNull<ffi::PyTypeObject>
-    where
-        F: Fn(Python) -> NonNull<ffi::PyTypeObject>,
-    {
-        if !self
-            .initialized
-            .compare_and_swap(false, true, Ordering::Acquire)
-        {
-            // We have to get the GIL before setting the value to the global!!!
-            let gil = Python::acquire_gil();
-            unsafe {
-                *self.value.get() = Some(constructor(gil.python()));
-            }
-        }
-        unsafe { (*self.value.get()).unwrap() }
-    }
-}
-
-// This is necessary for making static `LazyHeapType`s
-//
-// Type objects are shared between threads by the Python interpreter anyway, so it is no worse
-// to allow sharing on the Rust side too.
-unsafe impl Sync for LazyHeapType {}
-
 /// Lazy type object for PyClass
 #[doc(hidden)]
 pub struct LazyStaticType {
-    value: UnsafeCell<ffi::PyTypeObject>,
+    // Boxed because Python expects the type object to have a stable address.
-    initialized: AtomicBool,
+    value: GILOnceCell<Box<ffi::PyTypeObject>>,
+    // Threads which have begun initialization. Used for reentrant initialization detection.
+    initializing_threads: Mutex<Vec<ThreadId>>,
 }
 
 impl LazyStaticType {
     pub const fn new() -> Self {
         LazyStaticType {
-            value: UnsafeCell::new(ffi::PyTypeObject_INIT),
+            value: GILOnceCell::new(),
-            initialized: AtomicBool::new(false),
+            initializing_threads: const_mutex(Vec::new()),
         }
     }
 
-    pub fn get_or_init<T: PyClass>(&self) -> &ffi::PyTypeObject {
+    pub fn get_or_init<T: PyClass>(&self, py: Python) -> &ffi::PyTypeObject {
-        if !self
+        self.value
-            .initialized
+            .get_or_init(py, || {
-            .compare_and_swap(false, true, Ordering::Acquire)
+                {
-        {
+                    // Code evaluated at class init time, such as class attributes, might lead to
-            let gil = Python::acquire_gil();
+                    // recursive initalization of the type object if the class attribute is the same
-            let py = gil.python();
+                    // type as the class.
-            initialize_type_object::<T>(py, T::MODULE, unsafe { &mut *self.value.get() })
+                    //
-                .unwrap_or_else(|e| {
+                    // That could lead to all sorts of unsafety such as using incomplete type objects
-                    e.print(py);
+                    // to initialize class instances, so recursive initialization is prevented.
-                    panic!("An error occurred while initializing class {}", T::NAME)
+                    let thread_id = thread::current().id();
-                });
+                    let mut threads = self.initializing_threads.lock();
-        }
+                    if threads.contains(&thread_id) {
-        unsafe { &*self.value.get() }
+                        panic!("Recursive initialization of type_object for {}", T::NAME);
+                    } else {
+                        threads.push(thread_id)
+                    }
+                }
+
+                // Okay, not recursive initialization - can proceed safely.
+                let mut type_object = Box::new(ffi::PyTypeObject_INIT);
+
+                initialize_type_object::<T>(py, T::MODULE, type_object.as_mut()).unwrap_or_else(
+                    |e| {
+                        e.print(py);
+                        panic!("An error occurred while initializing class {}", T::NAME)
+                    },
+                );
+
+                // 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() = Vec::new();
+
+                type_object
+            })
+            .as_ref()
     }
 }
 

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 }
             }
 

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::<Foo>();
+
-    let bar_obj = py.get_type::<Bar>();
+    py.get_type::<SelfClassAttribute>();
-    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");
 }