Skip to content

Commit

Permalink
Introduce #[pyclass(unsendable)]
Browse files Browse the repository at this point in the history
  • Loading branch information
kngwyu committed Jun 29, 2020
1 parent 6335a7f commit e5cda5f
Show file tree
Hide file tree
Showing 10 changed files with 238 additions and 59 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- `#[pyclass(unsendable)]`. [#1009](https://github.com/PyO3/pyo3/pull/1009)

## [0.11.0] - 2020-06-28
### Added
Expand Down
6 changes: 6 additions & 0 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ If a custom class contains references to other Python objects that can be collec
* `extends=BaseType` - Use a custom base class. The base `BaseType` must implement `PyTypeInfo`.
* `subclass` - Allows Python classes to inherit from this class.
* `dict` - Adds `__dict__` support, so that the instances of this type have a dictionary containing arbitrary instance variables.
* `unsendable` - Making it safe to expose `!Send` structs to Python, where each object can be
accessed by another thread. A class marked with `unsendable` panics if accessed by another thread.
* `module="XXX"` - Set the name of the module the class will be shown as defined in. If not given, the class
will be a virtual member of the `builtins` module.

Expand Down Expand Up @@ -974,6 +976,10 @@ impl pyo3::class::proto_methods::HasProtoRegistry for MyClass {
&REGISTRY
}
}

impl pyo3::pyclass::PyClassSend for MyClass {
type ThreadChecker = pyo3::pyclass::ThreadCheckerStub<MyClass>;
}
# let gil = Python::acquire_gil();
# let py = gil.python();
# let cls = py.get_type::<MyClass>();
Expand Down
93 changes: 59 additions & 34 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,68 @@ For a detailed list of all changes, see [CHANGELOG.md](https://github.com/PyO3/p
### Stable Rust
PyO3 now supports the stable Rust toolchain. The minimum required version is 1.39.0.

### `#[pyclass]` structs must now be `Send`
### `#[pyclass]` structs must now be `Send` or `unsendable`
Because `#[pyclass]` structs can be sent between threads by the Python interpreter, they must implement
`Send` to guarantee thread safety. This bound was added in PyO3 `0.11.0`.

This may "break" some code which previously was accepted, even though it was unsound. To resolve this,
consider using types like `Arc` instead of `Rc`, `Mutex` instead of `RefCell`, and add `Send` to any
boxed closures stored inside the `#[pyclass]`.

Before:
```rust,compile_fail
use pyo3::prelude::*;
use std::rc::Rc;
use std::cell::RefCell;
#[pyclass]
struct NotThreadSafe {
shared_bools: Rc<RefCell<Vec<bool>>>,
closure: Box<Fn()>
}
```

After:
```rust
use pyo3::prelude::*;
use std::sync::{Arc, Mutex};
`Send` or declared as `unsendable`, by `#[pyclass(unsendable)]`.
Note that `unsendable` is added in PyO3 `0.11.1` and `Send` is always required in PyO3 `0.11.0`.

This may "break" some code which previously was accepted, even though it could be unsound.
There can be two fixes:

1. If you think that your `#[pyclass]` actually needs to be `Send`, then let's make it `Send`.
A common fix is using thread-safe types. E.g., `Arc` instead of `Rc`, `Mutex` instead of
`RefCell`, and `Box<dyn Send + T>` instead of `Box<dyn T>`.

Before:
```rust,compile_fail
use pyo3::prelude::*;
use std::rc::Rc;
use std::cell::RefCell;
#[pyclass]
struct NotThreadSafe {
shared_bools: Rc<RefCell<Vec<bool>>>,
closure: Box<dyn Fn()>
}
```
After:
```rust
use pyo3::prelude::*;
use std::sync::{Arc, Mutex};

#[pyclass]
struct ThreadSafe {
shared_bools: Arc<Mutex<Vec<bool>>>,
closure: Box<dyn Fn() + Send>
}
```
In situations where you cannot change your `#[pyclass]` to automatically implement `Send`
(e.g., when it contains a raw pointer), you can use `unsafe impl Send`. In such cases,
care should be taken to ensure the struct is actually thread safe.
See [the Rustnomicon](https://doc.rust-lang.org/nomicon/send-and-sync.html) for more.

2. If you don't want to make your `#[pyclass]` sendable, you can use `unsendable` flag.
A class marked with `unsendable` panics when accessed by another thread.

Before:
```rust,compile_fail
use pyo3::prelude::*;

#[pyclass]
struct Unsendable {
pointers: Vec<*mut std::os::raw::c_char>,
}
```

#[pyclass]
struct ThreadSafe {
shared_bools: Arc<Mutex<Vec<bool>>>,
closure: Box<Fn() + Send>
}
```
After:
```rust
use pyo3::prelude::*;

Or in situations where you cannot change your `#[pyclass]` to automatically implement `Send`
(e.g., when it contains a raw pointer), you can use `unsafe impl Send`.
In such cases, care should be taken to ensure the struct is actually thread safe.
See [the Rustnomicon](ttps://doc.rust-lang.org/nomicon/send-and-sync.html) for more.
#[pyclass(unsendable)]
struct Unsendable {
pointers: Vec<*mut std::os::raw::c_char>,
}
```

### All `PyObject` and `Py<T>` methods now take `Python` as an argument
Previously, a few methods such as `Object::get_refcnt` did not take `Python` as an argument (to
Expand Down
47 changes: 29 additions & 18 deletions pyo3-derive-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct PyClassArgs {
pub flags: Vec<syn::Expr>,
pub base: syn::TypePath,
pub has_extends: bool,
pub has_unsendable: bool,
pub module: Option<syn::LitStr>,
}

Expand All @@ -45,6 +46,7 @@ impl Default for PyClassArgs {
flags: vec![parse_quote! { 0 }],
base: parse_quote! { pyo3::PyAny },
has_extends: false,
has_unsendable: false,
}
}
}
Expand All @@ -60,7 +62,7 @@ impl PyClassArgs {
}
}

/// Match a single flag
/// Match a key/value flag
fn add_assign(&mut self, assign: &syn::ExprAssign) -> syn::Result<()> {
let syn::ExprAssign { left, right, .. } = assign;
let key = match &**left {
Expand Down Expand Up @@ -120,31 +122,27 @@ impl PyClassArgs {
Ok(())
}

/// Match a key/value flag
/// Match a single flag
fn add_path(&mut self, exp: &syn::ExprPath) -> syn::Result<()> {
let flag = exp.path.segments.first().unwrap().ident.to_string();
let path = match flag.as_str() {
"gc" => {
parse_quote! {pyo3::type_flags::GC}
}
"weakref" => {
parse_quote! {pyo3::type_flags::WEAKREF}
}
"subclass" => {
parse_quote! {pyo3::type_flags::BASETYPE}
}
"dict" => {
parse_quote! {pyo3::type_flags::DICT}
let mut push_flag = |flag| {
self.flags.push(syn::Expr::Path(flag));
};
match flag.as_str() {
"gc" => push_flag(parse_quote! {pyo3::type_flags::GC}),
"weakref" => push_flag(parse_quote! {pyo3::type_flags::WEAKREF}),
"subclass" => push_flag(parse_quote! {pyo3::type_flags::BASETYPE}),
"dict" => push_flag(parse_quote! {pyo3::type_flags::DICT}),
"unsendable" => {
self.has_unsendable = true;
}
_ => {
return Err(syn::Error::new_spanned(
&exp.path,
"Expected one of gc/weakref/subclass/dict",
"Expected one of gc/weakref/subclass/dict/unsendable",
))
}
};

self.flags.push(syn::Expr::Path(path));
Ok(())
}
}
Expand Down Expand Up @@ -386,6 +384,16 @@ fn impl_class(
quote! {}
};

let thread_checker = if attr.has_unsendable {
quote! { pyo3::pyclass::ThreadCheckerImpl<#cls> }
} else if attr.has_extends {
quote! {
pyo3::pyclass::ThreadCheckerInherited<#cls, <#cls as pyo3::type_object::PyTypeInfo>::BaseType>
}
} else {
quote! { pyo3::pyclass::ThreadCheckerStub<#cls> }
};

Ok(quote! {
unsafe impl pyo3::type_object::PyTypeInfo for #cls {
type Type = #cls;
Expand Down Expand Up @@ -424,6 +432,10 @@ fn impl_class(
type Target = pyo3::PyRefMut<'a, #cls>;
}

impl pyo3::pyclass::PyClassSend for #cls {
type ThreadChecker = #thread_checker;
}

#into_pyobject

#impl_inventory
Expand All @@ -433,7 +445,6 @@ fn impl_class(
#extra

#gc_impl

})
}

Expand Down
6 changes: 4 additions & 2 deletions src/derive_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::err::{PyErr, PyResult};
use crate::exceptions::TypeError;
use crate::instance::PyNativeType;
use crate::pyclass::PyClass;
use crate::pyclass::{PyClass, PyClassThreadChecker};
use crate::types::{PyAny, PyDict, PyModule, PyTuple};
use crate::{ffi, GILPool, IntoPy, PyCell, Python};
use std::cell::UnsafeCell;
Expand Down Expand Up @@ -157,18 +157,20 @@ impl ModuleDef {

/// Utilities for basetype
#[doc(hidden)]
pub trait PyBaseTypeUtils {
pub trait PyBaseTypeUtils: Sized {
type Dict;
type WeakRef;
type LayoutAsBase;
type BaseNativeType;
type ThreadChecker: PyClassThreadChecker<Self>;
}

impl<T: PyClass> PyBaseTypeUtils for T {
type Dict = T::Dict;
type WeakRef = T::WeakRef;
type LayoutAsBase = crate::pycell::PyCellInner<T>;
type BaseNativeType = T::BaseNativeType;
type ThreadChecker = T::ThreadChecker;
}

/// Utility trait to enable &PyClass as a pymethod/function argument
Expand Down
8 changes: 7 additions & 1 deletion src/pycell.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! Includes `PyCell` implementation.
use crate::conversion::{AsPyPointer, FromPyPointer, ToPyObject};
use crate::pyclass::{PyClass, PyClassThreadChecker};
use crate::pyclass_init::PyClassInitializer;
use crate::pyclass_slots::{PyClassDict, PyClassWeakRef};
use crate::type_object::{PyBorrowFlagLayout, PyLayout, PySizedLayout, PyTypeInfo};
use crate::types::PyAny;
use crate::{ffi, FromPy, PyClass, PyErr, PyNativeType, PyObject, PyResult, Python};
use crate::{ffi, FromPy, PyErr, PyNativeType, PyObject, PyResult, Python};
use std::cell::{Cell, UnsafeCell};
use std::fmt;
use std::mem::ManuallyDrop;
Expand Down Expand Up @@ -161,6 +162,7 @@ pub struct PyCell<T: PyClass> {
inner: PyCellInner<T>,
dict: T::Dict,
weakref: T::WeakRef,
thread_checker: T::ThreadChecker,
}

unsafe impl<T: PyClass> PyNativeType for PyCell<T> {}
Expand Down Expand Up @@ -227,6 +229,7 @@ impl<T: PyClass> PyCell<T> {
/// }
/// ```
pub fn try_borrow(&self) -> Result<PyRef<'_, T>, PyBorrowError> {
self.thread_checker.ensure();
let flag = self.inner.get_borrow_flag();
if flag == BorrowFlag::HAS_MUTABLE_BORROW {
Err(PyBorrowError { _private: () })
Expand Down Expand Up @@ -258,6 +261,7 @@ impl<T: PyClass> PyCell<T> {
/// assert!(c.try_borrow_mut().is_ok());
/// ```
pub fn try_borrow_mut(&self) -> Result<PyRefMut<'_, T>, PyBorrowMutError> {
self.thread_checker.ensure();
if self.inner.get_borrow_flag() != BorrowFlag::UNUSED {
Err(PyBorrowMutError { _private: () })
} else {
Expand Down Expand Up @@ -296,6 +300,7 @@ impl<T: PyClass> PyCell<T> {
/// }
/// ```
pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, PyBorrowError> {
self.thread_checker.ensure();
if self.inner.get_borrow_flag() == BorrowFlag::HAS_MUTABLE_BORROW {
Err(PyBorrowError { _private: () })
} else {
Expand Down Expand Up @@ -352,6 +357,7 @@ impl<T: PyClass> PyCell<T> {
let self_ = base as *mut Self;
(*self_).dict = T::Dict::new();
(*self_).weakref = T::WeakRef::new();
(*self_).thread_checker = T::ThreadChecker::new();
Ok(self_)
}
}
Expand Down
Loading

0 comments on commit e5cda5f

Please sign in to comment.