Add a Python::allow_thread_unsafe variant without a Send bound

[Tpt]

The python Python::allow_thread method allows to release the GIL when evaluating Rust code inside of a function exposed to the Python interpreter.
However, it requires the Send bound on the closure given for argument to it in order to make sure no object holding the GIL is an argument of this closure.
This creates a strong limitation on some code that is not thread safe (e.g. use std::rc::Rc) but "GIL safe" because it does not hold the GIL.
What about creating an unsafe version of the Python::allow_thread that would not enforce the Send bound and would be annotated as unsafe because it would require the caller to ensure the GIL is not leaked into the closure?

I (@Tpt) am willing to write an implementation PR if you think this addition is relevant to pyo3.

[davidhewitt]

How about having an opt-out mechanism for individual variables?

We could have unsafe fn assert_gil_safe<T>(t: T) -> GilSafe<T> which is just a newtype struct that implements Send and also Deref<Target = T>.

My reason for suggesting this alternative is that it's a very far-reaching unsafe to remove the bound completely - accidentally accessing a Python API with the GIL released can range from invisible raciness through to loud crashes.

[Tpt]

@davidhewitt Thank you for your feedback!

We could have unsafe fn assert_gil_safe<T>(t: T) -> GilSafe<T> which is just a newtype struct that implements Send and also Deref<Target = T>.

This sounds a bit scary to me. What if someone calls assert_gil_safe and then send the resulting GilSafe<T> to an other thread without much care? It seems to me it saves us from GIL concurrency bugs at the cost of thread safety bugs. What do you think about it?

[davidhewitt]

That's a fair point, that's not ideal either. Scrap my suggestion.

It's possible to implement allow_thread_unsafe using raw FFI. Perhaps we can start with an example for how users can do this themselves, rather than providing the API?

[Tpt]

It's possible to implement allow_thread_unsafe using raw FFI. Perhaps we can start with an example for how users can do this themselves, rather than providing the API?

Sure! If you want I can write something and do a pull request. Should I add an example inside of the examples directory, add something the the FAQ or do something else?

[davidhewitt]

I think the FAQ is a great place to start 👍

[mejrs]

I don't like the idea of unsafe_allow_threads (or telling users how to invent it themselves), because the unsafety is really quite tricky and unbounded here. If you get it wrong and do accidentally use Python types in the closure it has a high chance of "just working", but cause mysterious and hard to diagnose and reproduce bugs later. Such is the nature of data races 😔

[mejrs]

@Tpt do you have a concrete use case where you ran into this problem? Note that if you have a Rc<T>, you can still send &T into the closure, if the inner value is Send/Sync:

use pyo3::prelude::*;
use std::rc::Rc;

fn main() {
    Python::with_gil(|py| {
        let shared: Rc<[u8]> = vec![0; 25].into();

        let buf: &[u8] = &*shared;

        py.allow_threads(|| {
            println!("{:?}", buf);
        });
    });
}

Unfortunately this won't work if you have a structure using Rc internally, but these are less common.

[Tpt]

Unfortunately this won't work if you have a structure using Rc internally, but these are less common.

Yes, this is exactly my usecase. A structure with a lot of internal Rc with a costly initialization and that is then iterated on.

[mejrs]

@davidhewitt what would you think about just making a nightly-only feature that changes allow_threads to use our own auto trait? I think it's the best solution to this and the send_wrapper issue.

[davidhewitt]

This is now implemented as part of the nightly feature.

https://docs.rs/pyo3/latest/pyo3/marker/index.html#a-proper-implementation-using-an-auto-trait
https://pyo3.rs/v0.16.1/features.html#nightly