Introduce #[pyclass(unsendable)]

[kngwyu]

Now PyClass requires Send, but what should we do when we really don't want it sent to another thread?
Then we can use #[pyclass(unsendable)].
Inspired by send-wrapper, a class with unsendable panics when it is accessed by another thread.

#[pyclass(unsendable)]
struct Unsendable { ... }

unsendable = Unsendable()
def func():
    return unsendable.value()
import threading
# func is executed in anohter thread, thus causes panic
threading.Thread.start(target=func)

To enable this, I introduced the PyClassSend trait.

pub trait PyClassSend {
    type ThreadChecker: PyClassThreadChecker;
}

Without unsendable, we use ThreadCheckerStub as a ThreadChecker, which requires Send. Thus not unsendable class has to be Sendable.

impl pyo3::pyclass::PyClassSend for Class {
    type ThreadChecker = pyo3::pyclass::ThreadCheckerStub<Class>;
}

• Documentation

[programmerjake]

PyClassSend should probably be an unsafe trait, since if the wrong type is used in the impl, a !Send object can be accessed from a different thread.

[kngwyu]

@programmerjake
Good catch, thanks.

[kngwyu]

@programmerjake

PyClassSend should probably be an unsafe trait, since if the wrong type is used in the impl, a !Send object can be accessed from a different thread.

I think it's safe in that outer class cannot implement PyClassThreadChecker because it requires fn __private__(&self) -> crate::internal_tricks::PrivateMarker. And our all internal PyClassThreadCheckers are safe.

[davidhewitt]

I think the problem is that this "safe" code is effectively equivalent to unsafe impl Send:

impl pyo3::pyclass::PyClassSend for Unsendable {
    type ThreadChecker = pyo3::pyclass::ThreadCheckerStub<Sendable>;
}

I'm using Sendable's no-op thread checker to make Unsendable also have a no-op checker.

[birkenfeld]

Why does the stub have the type argument btw? (For the "real" checker it's used to format the panic message)

[kngwyu]

I think the problem is that this "safe" code is effectively equivalent to unsafe impl Send:

Ah, I understand, thanks. Then it have to be unsafe.

Why does the stub have the type argument btw?

For checking T: Send. There can be a better way, though.

[davidhewitt]

Having read through this implementation, I think it's very elegant. I had reservations about whether this was necessary for the complexity it adds. But the implementation is rather neat and should be zero-cost for sendable pyclasses after optimisations, so I have warmed to this idea.

For checking T: Send. There can be a better way, though.

You might be interested in this example for quote_spanned which asserts a class is Sync. It might be possible to do similar here as part of the #[pyclass] proc_macro for Send.

https://docs.rs/quote/1.0.7/quote/macro.quote_spanned.html#example

We could then just skip generating the check when unsendable is set.

Though I think doing that trick might mean that users who are manually implementing PyClass (if they really wanted to) would not have any check ensuring their type is Send.

[davidhewitt]

For the documentation: I'm thinking that there's a new chapter I'd like to write explaining with better examples the GIL, threading, Send and PyAny vs Py<> / PyObject in just a bit more detail.

So if you want, instead of writing the documentation in this PR, open an issue once this PR is merged and assign it to me.

[kngwyu]

For the documentation: I'm thinking that there's a new chapter I'd like to write explaining with better examples the GIL, threading, Send and PyAny vs Py<> / PyObject in just a bit more detail.

Well, it would be good
But anyway I think we need a short document on migration.md and I'll push it.

[kngwyu]

I changed the trait declaration to

pub trait PyClassSend {
    type ThreadChecker: PyClassThreadChecker<Self>;
}

for inhibiting invalid ThreadChecker.

[davidhewitt]

I think the PyClassSend trait still needs to be unsafe.

I think this would still be possible for a user to write:

impl PyClassThreadChecker<Unsendable> for ThreadCheckerStub<()> { ... }

And then the "safe" code above can just use ThreadCheckerStub<()> to skip the thread checks for the Unsendable struct.

[kngwyu]

I think this would still be possible for a user to write:

It's prevented by private_decl!. So we can say that it's practically safe, though I'm not sure if it's a good way to ensure safety.

[konstin]

This is great, thank you! 💯

[davidhewitt]

It's prevented by private_decl!. So we can say that it's practically safe, though I'm not sure if it's a good way to ensure safety.

Very true. Thanks, I'm satisfied!