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

Introduce #[pyclass(unsendable)] #1009

Merged
merged 2 commits into from
Jun 30, 2020
Merged
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
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)

### Changed
- Update `parking_lot` dependency to `0.11`. [#1010](https://github.com/PyO3/pyo3/pull/1010)
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 all object can be accessed
by multiple threads. A class marked with `unsendable` panics when 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
88 changes: 58 additions & 30 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,71 @@ 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`.
`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 must be `Send`able, then let's implement `Send`.
A common, safer way 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()>
}
```

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]`.
After:
```rust
use pyo3::prelude::*;
use std::sync::{Arc, Mutex};

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

#[pyclass]
struct NotThreadSafe {
shared_bools: Rc<RefCell<Vec<bool>>>,
closure: Box<Fn()>
}
```
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.

After:
```rust
use pyo3::prelude::*;
use std::sync::{Arc, Mutex};
2. If you think that your `#[pyclass]` should not be accessed by another thread, you can use
`unsendable` flag. A class marked with `unsendable` panics when accessed by another thread,
making it thread-safe to expose an unsendable object to the Python interpreter.

#[pyclass]
struct ThreadSafe {
shared_bools: Arc<Mutex<Vec<bool>>>,
closure: Box<Fn() + Send>
}
```
Before:
```rust,compile_fail
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](https://doc.rust-lang.org/nomicon/send-and-sync.html) for more.
#[pyclass]
struct Unsendable {
pointers: Vec<*mut std::os::raw::c_char>,
}
```

After:
```rust
use pyo3::prelude::*;

#[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