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 23, 2021
1 parent 947055d commit 3b6d347
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 19 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;
36 changes: 36 additions & 0 deletions src/impl_/not_send.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use std::marker::PhantomData;

use crate::Python;

/// A marker type that makes the type !Send.
/// Temporal hack until https://github.com/rust-lang/rust/issues/13231 is resolved.
#[derive(Copy, Clone)]
pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);

pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);


#[cfg(test)]
mod tests {
use super::NotSend;

// https://users.rust-lang.org/t/a-macro-to-assert-that-a-type-does-not-implement-trait-bounds/31179
macro_rules! assert_not_impl {
($x:ty, $($t:path),+ $(,)*) => {
const _: fn() -> () = || {
struct Check<T: ?Sized>(T);
trait AmbiguousIfImpl<A> { fn some_item() { } }

impl<T: ?Sized> AmbiguousIfImpl<()> for Check<T> { }
impl<T: ?Sized $(+ $t)*> AmbiguousIfImpl<u8> for Check<T> { }

<Check::<$x> as AmbiguousIfImpl<_>>::some_item()
};
};
}

#[test]
fn not_send() {
assert_not_impl!(NotSend, Send);
}
}
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
5 changes: 3 additions & 2 deletions 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::{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 Expand Up @@ -736,7 +737,7 @@ impl<'py> Python<'py> {
/// I.e., `Python<'static>` is always *really* unsafe.
#[inline]
pub unsafe fn assume_gil_acquired() -> Python<'py> {
Python(PhantomData)
Python(PhantomData, NOT_SEND)
}

/// Create a new pool for managing PyO3's owned references.
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);
})
}
12 changes: 12 additions & 0 deletions tests/ui/not_send.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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 `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 3b6d347

Please sign in to comment.