Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LazyStaticType::get_or_init returns an *mut instead of a & ref #994

Merged
merged 1 commit into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ unsafe impl pyo3::PyTypeInfo for MyClass {
const FLAGS: usize = 0;

#[inline]
fn type_object_raw(py: pyo3::Python) -> &'static pyo3::ffi::PyTypeObject {
fn type_object_raw(py: pyo3::Python) -> *mut pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>(py)
Expand Down
2 changes: 1 addition & 1 deletion pyo3-derive-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ fn impl_class(
const FLAGS: usize = #(#flags)|* | #extended;

#[inline]
fn type_object_raw(py: pyo3::Python) -> &'static pyo3::ffi::PyTypeObject {
fn type_object_raw(py: pyo3::Python) -> *mut pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>(py)
Expand Down
3 changes: 1 addition & 2 deletions src/freelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ where
T: PyTypeInfo + PyClassWithFreeList,
{
unsafe fn new(py: Python, subtype: *mut ffi::PyTypeObject) -> *mut Self::Layout {
let type_obj = Self::type_object_raw(py) as *const _ as _;
// if subtype is not equal to this type, we cannot use the freelist
if subtype == type_obj {
if subtype == Self::type_object_raw(py) {
if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list().pop() {
ffi::PyObject_Init(obj, subtype);
return obj as _;
Expand Down
4 changes: 2 additions & 2 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(crate) unsafe fn default_new<T: PyTypeInfo>(
// 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::type_object_raw(py);
if let Some(base_new) = base_tp.tp_new {
if let Some(base_new) = (*base_tp).tp_new {
return base_new(subtype, ptr::null_mut(), ptr::null_mut());
}
}
Expand Down Expand Up @@ -122,7 +122,7 @@ where
s => CString::new(s)?.into_raw(),
};

type_object.tp_base = T::BaseType::type_object_raw(py) as *const _ as _;
type_object.tp_base = T::BaseType::type_object_raw(py);

type_object.tp_name = match module_name {
Some(module_name) => CString::new(format!("{}.{}", module_name, T::NAME))?.into_raw(),
Expand Down
2 changes: 1 addition & 1 deletion src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<T: PyClass> PyClassInitializer<T> {
T: PyClass,
T::BaseLayout: PyBorrowFlagLayout<T::BaseType>,
{
unsafe { self.create_cell_from_subtype(py, T::type_object_raw(py) as *const _ as _) }
unsafe { self.create_cell_from_subtype(py, T::type_object_raw(py)) }
}

/// Create a new PyCell and initialize it given a typeobject `subtype`.
Expand Down
73 changes: 32 additions & 41 deletions src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,16 @@ pub unsafe trait PyTypeInfo: Sized {
type AsRefTarget: crate::PyNativeType;

/// PyTypeObject instance for this type.
fn type_object_raw(py: Python) -> &'static ffi::PyTypeObject;
fn type_object_raw(py: Python) -> *mut 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(object.py()) as *const _ as _,
) != 0
}
unsafe { ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object_raw(object.py())) != 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(object.py()) as *const _ as _ }
unsafe { (*object.as_ptr()).ob_type == Self::type_object_raw(object.py()) }
}
}

Expand All @@ -135,15 +130,15 @@ where
T: PyTypeInfo,
{
fn type_object(py: Python) -> &PyType {
unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as *const _ as _) }
unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as _) }
}
}

/// Lazy type object for PyClass
#[doc(hidden)]
pub struct LazyStaticType {
// Boxed because Python expects the type object to have a stable address.
value: GILOnceCell<Box<ffi::PyTypeObject>>,
value: GILOnceCell<*mut ffi::PyTypeObject>,
// Threads which have begun initialization. Used for reentrant initialization detection.
initializing_threads: Mutex<Vec<ThreadId>>,
}
Expand All @@ -156,42 +151,38 @@ impl LazyStaticType {
}
}

pub fn get_or_init<T: PyClass>(&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_id = thread::current().id();
let mut threads = self.initializing_threads.lock();
if threads.contains(&thread_id) {
panic!("Recursive initialization of type_object for {}", T::NAME);
} else {
threads.push(thread_id)
}
pub fn get_or_init<T: PyClass>(&self, py: Python) -> *mut 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_id = thread::current().id();
let mut threads = self.initializing_threads.lock();
if threads.contains(&thread_id) {
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);
// 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)
},
);
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();
// 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()
Box::into_raw(type_object)
})
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ macro_rules! pyobject_native_type_convert(
const MODULE: Option<&'static str> = $module;

#[inline]
fn type_object_raw(_py: Python) -> &'static $crate::ffi::PyTypeObject {
unsafe{ &$typeobject }
fn type_object_raw(_py: Python) -> *mut $crate::ffi::PyTypeObject {
// Create a very short lived mutable reference and directly
// cast it to a pointer: no mutable references can be aliasing
// because we hold the GIL.
unsafe { &mut $typeobject }
}

#[allow(unused_unsafe)]
Expand Down