err: don't use with_gil internally in PyErr::new()

This commit is contained in:
David Hewitt 2021-07-13 21:26:10 +01:00
parent dc58815143
commit c09947a61c
3 changed files with 73 additions and 20 deletions

View File

@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Add `indexmap` feature to add `ToPyObject`, `IntoPy` and `FromPyObject` implementations for `indexmap::IndexMap`. [#1728](https://github.com/PyO3/pyo3/pull/1728)
### Changed
- `PyErr::new` no longer acquires the Python GIL internally. [#1724](https://github.com/PyO3/pyo3/pull/1724)
### Fixed
- Fix regression in 0.14.0 rejecting usage of `#[doc(hidden)]` on structs and functions annotated with PyO3 macros. [#1722](https://github.com/PyO3/pyo3/pull/1722)

View File

@ -1,5 +1,9 @@
use crate::{
exceptions::PyBaseException, ffi, types::PyType, IntoPy, IntoPyPointer, Py, PyObject, Python,
exceptions::{PyBaseException, PyTypeError},
ffi,
type_object::PyTypeObject,
types::PyType,
AsPyPointer, IntoPy, IntoPyPointer, Py, PyObject, Python,
};
#[derive(Clone)]
@ -10,7 +14,11 @@ pub(crate) struct PyErrStateNormalized {
}
pub(crate) enum PyErrState {
Lazy {
LazyTypeAndValue {
ptype: fn(Python) -> &PyType,
pvalue: Box<dyn FnOnce(Python) -> PyObject + Send + Sync>,
},
LazyValue {
ptype: Py<PyType>,
pvalue: Box<dyn FnOnce(Python) -> PyObject + Send + Sync>,
},
@ -49,7 +57,19 @@ impl PyErrState {
py: Python,
) -> (*mut ffi::PyObject, *mut ffi::PyObject, *mut ffi::PyObject) {
match self {
PyErrState::Lazy { ptype, pvalue } => (
PyErrState::LazyTypeAndValue { ptype, pvalue } => {
let ty = ptype(py);
if unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) } == 0 {
Self::exceptions_must_derive_from_base_exception(py).into_ffi_tuple(py)
} else {
(
ptype(py).into_ptr(),
pvalue(py).into_ptr(),
std::ptr::null_mut(),
)
}
}
PyErrState::LazyValue { ptype, pvalue } => (
ptype.into_ptr(),
pvalue(py).into_ptr(),
std::ptr::null_mut(),
@ -66,4 +86,12 @@ impl PyErrState {
}) => (ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()),
}
}
#[inline]
pub(crate) fn exceptions_must_derive_from_base_exception(py: Python) -> Self {
PyErrState::LazyValue {
ptype: PyTypeError::type_object(py).into(),
pvalue: boxed_args("exceptions must derive from BaseException"),
}
}
}

View File

@ -81,7 +81,10 @@ impl PyErr {
T: PyTypeObject,
A: PyErrArguments + Send + Sync + 'static,
{
Python::with_gil(|py| PyErr::from_type(T::type_object(py), args))
PyErr::from_state(PyErrState::LazyTypeAndValue {
ptype: T::type_object,
pvalue: boxed_args(args),
})
}
/// Constructs a new error, with the usual lazy initialization of Python exceptions.
@ -97,7 +100,7 @@ impl PyErr {
return exceptions_must_derive_from_base_exception(ty.py());
}
PyErr::from_state(PyErrState::Lazy {
PyErr::from_state(PyErrState::LazyValue {
ptype: ty.into(),
pvalue: boxed_args(args),
})
@ -319,7 +322,7 @@ impl PyErr {
T: ToBorrowedObject,
{
exc.with_borrowed_ptr(py, |exc| unsafe {
ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), exc) != 0
ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), exc) != 0
})
}
@ -329,7 +332,7 @@ impl PyErr {
T: PyTypeObject,
{
unsafe {
ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), T::type_object(py).as_ptr()) != 0
ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), T::type_object(py).as_ptr()) != 0
}
}
@ -419,9 +422,12 @@ impl PyErr {
}
/// Returns borrowed reference to this Err's type
fn ptype_ptr(&self) -> *mut ffi::PyObject {
fn ptype_ptr(&self, py: Python) -> *mut ffi::PyObject {
match unsafe { &*self.state.get() } {
Some(PyErrState::Lazy { ptype, .. }) => ptype.as_ptr(),
// In lazy type case, normalize before returning ptype in case the type is not a valid
// exception type.
Some(PyErrState::LazyTypeAndValue { .. }) => self.normalized(py).ptype.as_ptr(),
Some(PyErrState::LazyValue { ptype, .. }) => ptype.as_ptr(),
Some(PyErrState::FfiTuple { ptype, .. }) => ptype.as_ptr(),
Some(PyErrState::Normalized(n)) => n.ptype.as_ptr(),
None => panic!("Cannot access exception type while normalizing"),
@ -554,10 +560,7 @@ pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> {
#[inline]
fn exceptions_must_derive_from_base_exception(py: Python) -> PyErr {
PyErr::from_state(PyErrState::Lazy {
ptype: exceptions::PyTypeError::type_object(py).into(),
pvalue: boxed_args("exceptions must derive from BaseException"),
})
PyErr::from_state(PyErrState::exceptions_must_derive_from_base_exception(py))
}
#[cfg(test)]
@ -567,13 +570,31 @@ mod tests {
use crate::{PyErr, Python};
#[test]
fn set_typeerror() {
let gil = Python::acquire_gil();
let py = gil.python();
let err: PyErr = exceptions::PyTypeError::new_err(());
fn set_valueerror() {
Python::with_gil(|py| {
let err: PyErr = exceptions::PyValueError::new_err("some exception message");
assert!(err.is_instance::<exceptions::PyValueError>(py));
err.restore(py);
assert!(PyErr::occurred(py));
drop(PyErr::fetch(py));
let err = PyErr::fetch(py);
assert!(err.is_instance::<exceptions::PyValueError>(py));
assert_eq!(err.to_string(), "ValueError: some exception message");
})
}
#[test]
fn invalid_error_type() {
Python::with_gil(|py| {
let err: PyErr = PyErr::new::<crate::types::PyString, _>(());
assert!(err.is_instance::<exceptions::PyTypeError>(py));
err.restore(py);
let err = PyErr::fetch(py);
assert!(err.is_instance::<exceptions::PyTypeError>(py));
assert_eq!(
err.to_string(),
"TypeError: exceptions must derive from BaseException"
);
})
}
#[test]