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

opt: improve handle_panic generated code #2074

Merged
merged 1 commit into from
Dec 24, 2021
Merged
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
opt: improve handle_panic generated code
  • Loading branch information
davidhewitt committed Dec 24, 2021
commit 5be5d775899122fbbeb4037631b2dbf0ab88136f
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- The `create_exception!` macro can now take an optional docstring. This docstring, if supplied, is visible to users (with `.__doc__` and `help()`) and
accompanies your error type in your crate's documentation.
- Improve performance and error messages for `#[derive(FromPyObject)]` for enums. [#2068](https://github.com/PyO3/pyo3/pull/2068)
- Tweak LLVM code for compile times for internal `handle_panic` helper. [#2073](https://github.com/PyO3/pyo3/pull/2073)

### Removed

11 changes: 3 additions & 8 deletions src/callback.rs
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ use crate::{GILPool, IntoPyPointer};
use crate::{IntoPy, PyObject, Python};
use std::any::Any;
use std::os::raw::c_int;
use std::panic::{AssertUnwindSafe, UnwindSafe};
use std::panic::UnwindSafe;
use std::{isize, panic};

/// A type which can be the return type of a python C-API callback
@@ -241,13 +241,8 @@ where
R: PyCallbackOutput,
{
let pool = GILPool::new();
let unwind_safe_py = AssertUnwindSafe(pool.python());
let panic_result = panic::catch_unwind(move || -> PyResult<_> {
let py = *unwind_safe_py;
body(py)
});

panic_result_into_callback_output(pool.python(), panic_result)
let py = pool.python();
panic_result_into_callback_output(py, panic::catch_unwind(move || -> PyResult<_> { body(py) }))
}

fn panic_result_into_callback_output<R>(
10 changes: 7 additions & 3 deletions src/gil.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,8 @@

//! Interaction with Python's global interpreter lock

use crate::{ffi, internal_tricks::Unsendable, Python};
use crate::impl_::not_send::{NotSend, NOT_SEND};
use crate::{ffi, Python};
use parking_lot::{const_mutex, Mutex, Once};
use std::cell::{Cell, RefCell};
use std::{
@@ -157,6 +158,7 @@ where
pub struct GILGuard {
gstate: ffi::PyGILState_STATE,
pool: ManuallyDrop<Option<GILPool>>,
_not_send: NotSend,
}

impl GILGuard {
@@ -230,6 +232,7 @@ impl GILGuard {
GILGuard {
gstate,
pool: ManuallyDrop::new(pool),
_not_send: NOT_SEND,
}
}
}
@@ -332,7 +335,7 @@ pub struct GILPool {
/// Initial length of owned objects and anys.
/// `Option` is used since TSL can be broken when `new` is called from `atexit`.
start: Option<usize>,
no_send: Unsendable,
_not_send: NotSend,
}

impl GILPool {
@@ -351,11 +354,12 @@ impl GILPool {
POOL.update_counts(Python::assume_gil_acquired());
GILPool {
start: OWNED_OBJECTS.try_with(|o| o.borrow().len()).ok(),
no_send: Unsendable::default(),
_not_send: NOT_SEND,
}
}

/// Gets the Python token associated with this [`GILPool`].
#[inline]
pub fn python(&self) -> Python {
unsafe { Python::assume_gil_acquired() }
}
1 change: 1 addition & 0 deletions src/impl_.rs
Original file line number Diff line number Diff line change
@@ -8,3 +8,4 @@ pub mod deprecations;
pub mod freelist;
#[doc(hidden)]
pub mod frompyobject;
pub(crate) mod not_send;
9 changes: 9 additions & 0 deletions src/impl_/not_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use std::marker::PhantomData;

use crate::Python;

/// A marker type that makes the type !Send.
/// Workaround for lack of !Send on stable (https://github.com/rust-lang/rust/issues/68318).
pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);

pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);
6 changes: 0 additions & 6 deletions src/internal_tricks.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
use crate::ffi::{Py_ssize_t, PY_SSIZE_T_MAX};
use std::ffi::{CStr, CString};
use std::marker::PhantomData;
use std::rc::Rc;

/// A marker type that makes the type !Send.
/// Temporal hack until https://github.com/rust-lang/rust/issues/13231 is resolved.
pub(crate) type Unsendable = PhantomData<Rc<()>>;

pub struct PrivateMarker;

1 change: 1 addition & 0 deletions src/panic.rs
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ Python interpreter to exit.

impl PanicException {
// Try to format the error in the same way panic does
#[cold]
pub(crate) fn from_panic_payload(payload: Box<dyn Any + Send + 'static>) -> PyErr {
if let Some(string) = payload.downcast_ref::<String>() {
Self::new_err((string.clone(),))
3 changes: 2 additions & 1 deletion src/python.rs
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@

use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::gil::{self, GILGuard, GILPool};
use crate::impl_::not_send::NotSend;
use crate::type_object::{PyTypeInfo, PyTypeObject};
use crate::types::{PyAny, PyDict, PyModule, PyType};
use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom};
@@ -184,7 +185,7 @@ impl PartialOrd<(u8, u8, u8)> for PythonVersionInfo<'_> {
/// [`Py::clone_ref`]: crate::Py::clone_ref
/// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
#[derive(Copy, Clone)]
pub struct Python<'py>(PhantomData<&'py GILGuard>);
pub struct Python<'py>(PhantomData<(&'py GILGuard, NotSend)>);

impl Python<'_> {
/// Acquires the global interpreter lock, allowing access to the Python interpreter. The
1 change: 1 addition & 0 deletions tests/test_compile_error.rs
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@ fn _test_compile_errors() {
t.compile_fail("tests/ui/invalid_pymodule_args.rs");
t.compile_fail("tests/ui/missing_clone.rs");
t.compile_fail("tests/ui/reject_generics.rs");
t.compile_fail("tests/ui/not_send.rs");

tests_rust_1_49(&t);
tests_rust_1_55(&t);
11 changes: 11 additions & 0 deletions tests/ui/not_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use pyo3::prelude::*;

fn test_not_send_allow_threads(py: Python) {
py.allow_threads(|| { drop(py); });
}

fn main() {
Python::with_gil(|py| {
test_not_send_allow_threads(py);
})
}
14 changes: 14 additions & 0 deletions tests/ui/not_send.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0277]: `*mut pyo3::Python<'static>` cannot be shared between threads safely
--> tests/ui/not_send.rs:4:8
|
4 | py.allow_threads(|| { drop(py); });
| ^^^^^^^^^^^^^ `*mut pyo3::Python<'static>` cannot be shared between threads safely
|
= help: within `pyo3::Python<'_>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>`
= note: required because it appears within the type `PhantomData<*mut pyo3::Python<'static>>`
= note: required because it appears within the type `pyo3::impl_::not_send::NotSend`
= note: required because it appears within the type `(&GILGuard, pyo3::impl_::not_send::NotSend)`
= note: required because it appears within the type `PhantomData<(&GILGuard, pyo3::impl_::not_send::NotSend)>`
= note: required because it appears within the type `pyo3::Python<'_>`
= note: required because of the requirements on the impl of `Send` for `&pyo3::Python<'_>`
= note: required because it appears within the type `[closure@$DIR/tests/ui/not_send.rs:4:22: 4:38]`