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

require #[pyclass] to be Sync #4566

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
14 changes: 14 additions & 0 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2293,7 +2293,21 @@ impl<'a> PyClassImplsBuilder<'a> {
}
});

let assertions = if attr.options.unsendable.is_some() {
TokenStream::new()
} else {
quote_spanned! {
cls.span() =>
const _: () = {
use #pyo3_path::impl_::pyclass::*;
assert_pyclass_sync::<#cls, { IsSync::<#cls>::VALUE }>();
};
}
};

Ok(quote! {
#assertions

#pyclass_base_type_impl

impl #pyo3_path::impl_::pyclass::PyClassImpl for #cls {
Expand Down
4 changes: 4 additions & 0 deletions src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ pub struct Coroutine {
waker: Option<Arc<AsyncioWaker>>,
}

// Safety: `Coroutine` is allowed to be `Sync` even though the future is not,
// because the future is polled with `&mut self` receiver
unsafe impl Sync for Coroutine {}

impl Coroutine {
/// Wrap a future into a Python coroutine.
///
Expand Down
61 changes: 5 additions & 56 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ use std::{
thread,
};

mod assertions;
mod lazy_type_object;
mod probes;

pub use assertions::*;
pub use lazy_type_object::LazyTypeObject;
pub use probes::*;

/// Gets the offset of the dictionary from the start of the object in bytes.
#[inline]
Expand Down Expand Up @@ -1419,62 +1424,6 @@ impl<ClassT: PyClass, FieldT, Offset: OffsetCalculator<ClassT, FieldT>>
}
}

/// Trait used to combine with zero-sized types to calculate at compile time
/// some property of a type.
///
/// The trick uses the fact that an associated constant has higher priority
/// than a trait constant, so we can use the trait to define the false case.
///
/// The true case is defined in the zero-sized type's impl block, which is
/// gated on some property like trait bound or only being implemented
/// for fixed concrete types.
pub trait Probe {
const VALUE: bool = false;
}

macro_rules! probe {
($name:ident) => {
pub struct $name<T>(PhantomData<T>);
impl<T> Probe for $name<T> {}
};
}

probe!(IsPyT);

impl<T> IsPyT<Py<T>> {
pub const VALUE: bool = true;
}

probe!(IsToPyObject);

impl<T: ToPyObject> IsToPyObject<T> {
pub const VALUE: bool = true;
}

probe!(IsIntoPy);

impl<T: IntoPy<crate::PyObject>> IsIntoPy<T> {
pub const VALUE: bool = true;
}

probe!(IsIntoPyObjectRef);

impl<'a, 'py, T: 'a> IsIntoPyObjectRef<T>
where
&'a T: IntoPyObject<'py>,
{
pub const VALUE: bool = true;
}

probe!(IsIntoPyObject);

impl<'py, T> IsIntoPyObject<T>
where
T: IntoPyObject<'py>,
{
pub const VALUE: bool = true;
}

/// ensures `obj` is not mutably aliased
#[inline]
unsafe fn ensure_no_mutable_alias<'py, ClassT: PyClass>(
Expand Down
40 changes: 40 additions & 0 deletions src/impl_/pyclass/assertions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/// Helper function that can be used at compile time to emit a diagnostic if
/// the type does not implement `Sync` when it should.
///
/// The mere act of invoking this function will cause the diagnostic to be
/// emitted if `T` does not implement `Sync` when it should.
///
/// The additional `const IS_SYNC: bool` parameter is used to allow the custom
/// diagnostic to be emitted; if `PyClassSync`
pub const fn assert_pyclass_sync<T, const IS_SYNC: bool>()
where
T: PyClassSync<IS_SYNC> + Sync,
{
}

#[cfg_attr(
diagnostic_namespace,
diagnostic::on_unimplemented(
message = "the trait `Sync` is not implemented for `{Self}`",
label = "needs to implement `Sync` to be `#[pyclass]`",
note = "to opt-out of threading support, use `#[pyclass(unsendable)]`",
note = "see <TODO INSERT PYO3 GUIDE> for more information",
)
)]
pub trait PyClassSync<const IS_SYNC: bool>: private::Sealed<IS_SYNC> {}

mod private {
pub trait Sealed<const IS_SYNC: bool> {}
impl<T> Sealed<true> for T {}
#[cfg(not(diagnostic_namespace))]
impl<T> Sealed<false> for T {}
}

// If `true` is passed for the const parameter, then the diagnostic will
// not be emitted.
impl<T> PyClassSync<true> for T {}

// Without `diagnostic_namespace`, the trait bound is not useful, so we add
// an implementation for `false`` to avoid a useless diagnostic.
#[cfg(not(diagnostic_namespace))]
impl<T> PyClassSync<false> for T {}
65 changes: 65 additions & 0 deletions src/impl_/pyclass/probes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use std::marker::PhantomData;

use crate::{conversion::IntoPyObject, IntoPy, Py, ToPyObject};

/// Trait used to combine with zero-sized types to calculate at compile time
/// some property of a type.
///
/// The trick uses the fact that an associated constant has higher priority
/// than a trait constant, so we can use the trait to define the false case.
///
/// The true case is defined in the zero-sized type's impl block, which is
/// gated on some property like trait bound or only being implemented
/// for fixed concrete types.
pub trait Probe {
const VALUE: bool = false;
}

macro_rules! probe {
($name:ident) => {
pub struct $name<T>(PhantomData<T>);
impl<T> Probe for $name<T> {}
};
}

probe!(IsPyT);

impl<T> IsPyT<Py<T>> {
pub const VALUE: bool = true;
}

probe!(IsToPyObject);

impl<T: ToPyObject> IsToPyObject<T> {
pub const VALUE: bool = true;
}

probe!(IsIntoPy);

impl<T: IntoPy<crate::PyObject>> IsIntoPy<T> {
pub const VALUE: bool = true;
}

probe!(IsIntoPyObjectRef);

impl<'a, 'py, T: 'a> IsIntoPyObjectRef<T>
where
&'a T: IntoPyObject<'py>,
{
pub const VALUE: bool = true;
}

probe!(IsIntoPyObject);

impl<'py, T> IsIntoPyObject<T>
where
T: IntoPyObject<'py>,
{
pub const VALUE: bool = true;
}

probe!(IsSync);

impl<T: Sync> IsSync<T> {
pub const VALUE: bool = true;
}
48 changes: 25 additions & 23 deletions tests/ui/pyclass_send.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
use pyo3::prelude::*;
use std::rc::Rc;
use std::os::raw::c_void;

#[pyclass]
struct NotThreadSafe {
data: Rc<i32>,
}

fn main() {
let obj = Python::with_gil(|py| {
Bound::new(py, NotThreadSafe { data: Rc::new(5) })
.unwrap()
.unbind()
});

std::thread::spawn(move || {
Python::with_gil(|py| {
// Uh oh, moved Rc to a new thread!
let c = obj.bind(py).downcast::<NotThreadSafe>().unwrap();

assert_eq!(*c.borrow().data, 5);
})
})
.join()
.unwrap();
}
struct NotSyncNotSend(*mut c_void);

#[pyclass]
struct SendNotSync(*mut c_void);
unsafe impl Send for SendNotSync {}

#[pyclass]
struct SyncNotSend(*mut c_void);
unsafe impl Sync for SyncNotSend {}

// None of the `unsendable` forms below should fail to compile

#[pyclass(unsendable)]
struct NotSyncNotSendUnsendable(*mut c_void);

#[pyclass(unsendable)]
struct SendNotSyncUnsendable(*mut c_void);
unsafe impl Send for SendNotSyncUnsendable {}

#[pyclass(unsendable)]
struct SyncNotSendUnsendable(*mut c_void);
unsafe impl Sync for SyncNotSendUnsendable {}

fn main() {}
Loading
Loading