luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

change PyModule::add_class to return an error if class creation fails

This commit is contained in:
David Hewitt 2023-02-11 20:51:52 +00:00
parent d0d944cab4
commit 00ddd21535
10 changed files with 191 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
benches/bench_pyclass.rs
View File

@ -1,5 +1,5 @@
use criterion::{criterion_group, criterion_main, Criterion};
use pyo3::{impl_::pyclass::LazyStaticType, prelude::*};
use pyo3::{impl_::pyclass::LazyTypeObject, prelude::*};
/// This is a feature-rich class instance used to benchmark various parts of the pyclass lifecycle.
#[pyclass]
@ -31,8 +31,8 @@ pub fn first_time_init(b: &mut criterion::Bencher<'_>) {
b.iter(|| {
// This is using an undocumented internal PyO3 API to measure pyclass performance; please
// don't use this in your own code!
let ty = LazyStaticType::new();
ty.get_or_init::<MyClass>(py);
let ty = LazyTypeObject::<MyClass>::new();
ty.get_or_init(py);
});
});
}

11
guide/src/class.md
View File

@ -979,9 +979,8 @@ unsafe impl pyo3::type_object::PyTypeInfo for MyClass {
const MODULE: ::std::option::Option<&'static str> = ::std::option::Option::None;
#[inline]
fn type_object_raw(py: pyo3::Python<'_>) -> *mut pyo3::ffi::PyTypeObject {
use pyo3::impl_::pyclass::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>(py)
<Self as pyo3::impl_::pyclass::PyClassImpl>::lazy_type_object()
.get_or_init(py)
}
}
@ -1033,6 +1032,12 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
static INTRINSIC_ITEMS: PyClassItems = PyClassItems { slots: &[], methods: &[] };
PyClassItemsIter::new(&INTRINSIC_ITEMS, collector.py_methods())
}
fn lazy_type_object() -> &'static pyo3::impl_::pyclass::LazyTypeObject<MyClass> {
use pyo3::impl_::pyclass::LazyTypeObject;
static TYPE_OBJECT: LazyTypeObject<MyClass> = LazyTypeObject::new();
&TYPE_OBJECT
}
}
# Python::with_gil(|py| {

1
newsfragments/2947.changed.md Normal file
View File

@ -0,0 +1 @@
Improve exception raised when creating `#[pyclass]` type object fails during module import.

11
pyo3-macros-backend/src/pyclass.rs
View File

@ -756,9 +756,8 @@ fn impl_pytypeinfo(
fn type_object_raw(py: _pyo3::Python<'_>) -> *mut _pyo3::ffi::PyTypeObject {
#deprecations
use _pyo3::impl_::pyclass::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_or_init::<Self>(py)
<#cls as _pyo3::impl_::pyclass::PyClassImpl>::lazy_type_object()
.get_or_init(py)
}
}
}
@ -1038,6 +1037,12 @@ impl<'a> PyClassImplsBuilder<'a> {
#dict_offset
#weaklist_offset
fn lazy_type_object() -> &'static _pyo3::impl_::pyclass::LazyTypeObject<Self> {
use _pyo3::impl_::pyclass::LazyTypeObject;
static TYPE_OBJECT: LazyTypeObject<#cls> = LazyTypeObject::new();
&TYPE_OBJECT
}
}
#[doc(hidden)]

8
src/impl_/pyclass.rs
View File

@ -15,8 +15,8 @@ use std::{
thread,
};
mod lazy_static_type;
pub use lazy_static_type::LazyStaticType;
mod lazy_type_object;
pub use lazy_type_object::LazyTypeObject;
/// Gets the offset of the dictionary from the start of the object in bytes.
#[inline]
@ -137,7 +137,7 @@ unsafe impl Sync for PyClassItems {}
///
/// Users are discouraged from implementing this trait manually; it is a PyO3 implementation detail
/// and may be changed at any time.
pub trait PyClassImpl: Sized {
pub trait PyClassImpl: Sized + 'static {
/// Class doc string
const DOC: &'static str = "\0";
@ -194,6 +194,8 @@ pub trait PyClassImpl: Sized {
fn weaklist_offset() -> Option<ffi::Py_ssize_t> {
None
}
fn lazy_type_object() -> &'static LazyTypeObject<Self>;
}
/// Iterator used to process all class items during type instantiation.

121
src/impl_/pyclass/lazy_static_type.rs → src/impl_/pyclass/lazy_type_object.rs
View File

@ -1,22 +1,25 @@
use std::{
borrow::Cow,
ffi::CStr,
marker::PhantomData,
thread::{self, ThreadId},
};
use parking_lot::{const_mutex, Mutex};
use crate::{
ffi, once_cell::GILOnceCell, pyclass::create_type_object, IntoPyPointer, PyClass,
PyMethodDefType, PyObject, PyResult, Python,
exceptions::PyRuntimeError, ffi, once_cell::GILOnceCell, pyclass::create_type_object,
IntoPyPointer, PyClass, PyErr, PyMethodDefType, PyObject, PyResult, Python,
};
use super::PyClassItemsIter;
/// Lazy type object for PyClass.
#[doc(hidden)]
pub struct LazyStaticType {
// Boxed because Python expects the type object to have a stable address.
pub struct LazyTypeObject<T>(LazyTypeObjectInner, PhantomData<T>);
// Non-generic inner of LazyTypeObject to keep code size down
struct LazyTypeObjectInner {
value: GILOnceCell<*mut ffi::PyTypeObject>,
// Threads which have begun initialization of the `tp_dict`. Used for
// reentrant initialization detection.
@ -24,31 +27,64 @@ pub struct LazyStaticType {
tp_dict_filled: GILOnceCell<PyResult<()>>,
}
impl LazyStaticType {
/// Creates an uninitialized `LazyStaticType`.
impl<T> LazyTypeObject<T> {
/// Creates an uninitialized `LazyTypeObject`.
pub const fn new() -> Self {
LazyStaticType {
value: GILOnceCell::new(),
initializing_threads: const_mutex(Vec::new()),
tp_dict_filled: GILOnceCell::new(),
}
LazyTypeObject(
LazyTypeObjectInner {
value: GILOnceCell::new(),
initializing_threads: const_mutex(Vec::new()),
tp_dict_filled: GILOnceCell::new(),
},
PhantomData,
)
}
}
impl<T: PyClass> LazyTypeObject<T> {
/// Gets the type object contained by this `LazyTypeObject`, initializing it if needed.
pub fn get_or_init(&self, py: Python<'_>) -> *mut ffi::PyTypeObject {
self.get_or_try_init(py).unwrap_or_else(|err| {
err.print(py);
panic!("failed to create type object for {}", T::NAME)
})
}
/// Gets the type object contained by this `LazyStaticType`, initializing it if needed.
pub fn get_or_init<T: PyClass>(&self, py: Python<'_>) -> *mut ffi::PyTypeObject {
fn inner<T: PyClass>() -> *mut ffi::PyTypeObject {
/// Fallible version of the above.
pub(crate) fn get_or_try_init(&self, py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject> {
fn inner<T: PyClass>() -> PyResult<*mut ffi::PyTypeObject> {
// Safety: `py` is held by the caller of `get_or_init`.
let py = unsafe { Python::assume_gil_acquired() };
create_type_object::<T>(py)
}
// Uses explicit GILOnceCell::get_or_init::<fn() -> *mut ffi::PyTypeObject> monomorphization
// so that only this one monomorphization is instantiated (instead of one closure monormization for each T).
let type_object = *self
.value
.get_or_init::<fn() -> *mut ffi::PyTypeObject>(py, inner::<T>);
self.ensure_init(py, type_object, T::NAME, T::items_iter());
type_object
self.0
.get_or_try_init(py, inner::<T>, T::NAME, T::items_iter())
}
}
impl LazyTypeObjectInner {
fn get_or_try_init(
&self,
py: Python<'_>,
init: fn() -> PyResult<*mut ffi::PyTypeObject>,
name: &str,
items_iter: PyClassItemsIter,
) -> PyResult<*mut ffi::PyTypeObject> {
(|| -> PyResult<_> {
// Uses explicit GILOnceCell::get_or_init::<fn() -> *mut ffi::PyTypeObject> monomorphization
// so that this code is only monomorphized once, instead of for every T.
let type_object = *self.value.get_or_try_init(py, init)?;
self.ensure_init(py, type_object, name, items_iter)?;
Ok(type_object)
})()
.map_err(|err| {
wrap_in_runtime_error(
py,
err,
format!("An error occurred while initializing class {}", name),
)
})
}
fn ensure_init(
@ -57,7 +93,7 @@ impl LazyStaticType {
type_object: *mut ffi::PyTypeObject,
name: &str,
items_iter: PyClassItemsIter,
) {
) -> PyResult<()> {
// We might want to fill the `tp_dict` with python instances of `T`
// itself. In order to do so, we must first initialize the type object
// with an empty `tp_dict`: now we can create instances of `T`.
@ -71,7 +107,7 @@ impl LazyStaticType {
if self.tp_dict_filled.get(py).is_some() {
// `tp_dict` is already filled: ok.
return;
return Ok(());
}
let thread_id = thread::current().id();
@ -80,7 +116,7 @@ impl LazyStaticType {
if threads.contains(&thread_id) {
// Reentrant call: just return the type object, even if the
// `tp_dict` is not filled yet.
return;
return Ok(());
}
threads.push(thread_id);
}
@ -113,12 +149,17 @@ impl LazyStaticType {
match (attr.meth.0)(py) {
Ok(val) => items.push((key, val)),
Err(e) => panic!(
"An error occurred while initializing `{}.{}`: {}",
name,
attr.name.trim_end_matches('\0'),
e
),
Err(err) => {
return Err(wrap_in_runtime_error(
py,
err,
format!(
"An error occurred while initializing `{}.{}`",
name,
attr.name.trim_end_matches('\0')
),
))
}
}
}
}
@ -137,9 +178,14 @@ impl LazyStaticType {
});
if let Err(err) = result {
err.clone_ref(py).print(py);
panic!("An error occurred while initializing `{}.__dict__`", name);
return Err(wrap_in_runtime_error(
py,
err.clone_ref(py),
format!("An error occurred while initializing `{}.__dict__`", name),
));
}
Ok(())
}
}
@ -157,5 +203,12 @@ fn initialize_tp_dict(
Ok(())
}
// This is necessary for making static `LazyStaticType`s
unsafe impl Sync for LazyStaticType {}
// This is necessary for making static `LazyTypeObject`s
unsafe impl<T> Sync for LazyTypeObject<T> {}
#[cold]
fn wrap_in_runtime_error(py: Python<'_>, err: PyErr, message: String) -> PyErr {
let runtime_err = PyRuntimeError::new_err(message);
runtime_err.set_cause(py, Some(err));
runtime_err
}

13
src/pyclass/create_type_object.rs
View File

@ -15,11 +15,11 @@ use std::{
ptr,
};
pub(crate) fn create_type_object<T>(py: Python<'_>) -> *mut ffi::PyTypeObject
pub(crate) fn create_type_object<T>(py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject>
where
T: PyClass,
{
match unsafe {
unsafe {
PyTypeBuilder::default()
.type_doc(T::DOC)
.offsets(T::dict_offset(), T::weaklist_offset())
@ -30,9 +30,6 @@ where
.set_is_sequence(T::IS_SEQUENCE)
.class_items(T::items_iter())
.build(py, T::NAME, T::MODULE, std::mem::size_of::<T::Layout>())
} {
Ok(type_object) => type_object,
Err(e) => type_object_creation_failed(py, e, T::NAME),
}
}
@ -386,12 +383,6 @@ impl PyTypeBuilder {
}
}
#[cold]
fn type_object_creation_failed(py: Python<'_>, e: PyErr, name: &str) -> ! {
e.print(py);
panic!("An error occurred while initializing class {}", name)
}
fn py_class_doc(class_doc: &str) -> Option<*mut c_char> {
match class_doc {
"\0" => None,

7
src/types/module.rs
View File

@ -7,7 +7,7 @@ use crate::err::{PyErr, PyResult};
use crate::exceptions;
use crate::ffi;
use crate::pyclass::PyClass;
use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString};
use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString, PyType};
use crate::{AsPyPointer, IntoPy, Py, PyObject, Python};
use std::ffi::{CStr, CString};
use std::str;
@ -294,7 +294,10 @@ impl PyModule {
where
T: PyClass,
{
self.add(T::NAME, T::type_object(self.py()))
let py = self.py();
self.add(T::NAME, unsafe {
PyType::from_type_ptr(py, T::lazy_type_object().get_or_try_init(py)?)
})
}
/// Adds a function or a (sub)module to a module, using the functions name as name.

55
tests/test_class_attributes.rs
View File

@ -1,6 +1,6 @@
#![cfg(feature = "macros")]
use pyo3::{exceptions::PyValueError, prelude::*};
use pyo3::{exceptions::PyValueError, prelude::*, types::PyString};
mod common;
@ -96,10 +96,7 @@ fn recursive_class_attributes() {
}
#[test]
#[should_panic(
expected = "An error occurred while initializing `BrokenClass.fails_to_init`: \
ValueError: failed to create class attribute"
)]
#[cfg(panic = "unwind")]
fn test_fallible_class_attribute() {
#[pyclass]
struct BrokenClass;
@ -113,6 +110,52 @@ fn test_fallible_class_attribute() {
}
Python::with_gil(|py| {
py.get_type::<BrokenClass>();
let stderr = CaptureStdErr::new(py).unwrap();
assert!(std::panic::catch_unwind(|| py.get_type::<BrokenClass>()).is_err());
assert_eq!(
stderr.reset().unwrap().trim(),
"\
ValueError: failed to create class attribute
The above exception was the direct cause of the following exception:
RuntimeError: An error occurred while initializing `BrokenClass.fails_to_init`
The above exception was the direct cause of the following exception:
RuntimeError: An error occurred while initializing class BrokenClass"
)
})
}
struct CaptureStdErr<'py> {
oldstderr: &'py PyAny,
string_io: &'py PyAny,
}
impl<'py> CaptureStdErr<'py> {
fn new(py: Python<'py>) -> PyResult<Self> {
let sys = py.import("sys")?;
let oldstderr = sys.getattr("stderr")?;
let string_io = py.import("io")?.getattr("StringIO")?.call0()?;
sys.setattr("stderr", string_io)?;
Ok(Self {
oldstderr,
string_io,
})
}
fn reset(self) -> PyResult<String> {
let py = self.string_io.py();
let payload = self
.string_io
.getattr("getvalue")?
.call0()?
.downcast::<PyString>()?
.to_str()?
.to_owned();
let sys = py.import("sys")?;
sys.setattr("stderr", self.oldstderr)?;
Ok(payload)
}
}

23
tests/test_inheritance.rs
View File

@ -339,3 +339,26 @@ fn test_subclass_ref_counts() {
);
})
}
#[test]
#[cfg(not(Py_LIMITED_API))]
fn module_add_class_inherit_bool_fails() {
use pyo3::types::PyBool;
#[pyclass(extends = PyBool)]
struct ExtendsBool;
Python::with_gil(|py| {
let m = PyModule::new(py, "test_module").unwrap();
let err = m.add_class::<ExtendsBool>().unwrap_err();
assert_eq!(
err.to_string(),
"RuntimeError: An error occurred while initializing class ExtendsBool"
);
assert_eq!(
err.cause(py).unwrap().to_string(),
"TypeError: type 'bool' is not an acceptable base type"
);
})
}