Skip to content

Commit

Permalink
opt: improve handle_panic generated code
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Dec 24, 2021
1 parent 947055d commit 5be5d77
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 3 additions & 8 deletions src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>(
Expand Down
10 changes: 7 additions & 3 deletions src/gil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -157,6 +158,7 @@ where
pub struct GILGuard {
gstate: ffi::PyGILState_STATE,
pool: ManuallyDrop<Option<GILPool>>,
_not_send: NotSend,
}

impl GILGuard {
Expand Down Expand Up @@ -230,6 +232,7 @@ impl GILGuard {
GILGuard {
gstate,
pool: ManuallyDrop::new(pool),
_not_send: NOT_SEND,
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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() }
}
Expand Down
1 change: 1 addition & 0 deletions src/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),))
Expand Down
3 changes: 2 additions & 1 deletion src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/test_compile_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
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]`

0 comments on commit 5be5d77

Please sign in to comment.