Supporting async for #[pyfunction] and #[pymethods]

[awestlake87]

Here's a first attempt at supporting async functions via pyo3-asyncio in the #[pyfunction] and #[pymethods] proc macro. Currently this PR adds support for async #[pyfunction] but not #[pymethods] just yet because it requires recognizing that functions returning impl Future should be treated the same as async functions.

I kept things simple to get a good baseline for discussion. Currently there's no way to select a runtime, I just assume async-std. I'm not sure how the #[pyfunction] attribute should go about selecting one just yet. Also, there's currently no support for impl Future, which is the desugared form of async fn. impl Future could be useful in cases where the 'static lifetime of the returned future cannot be inferred correctly.

The reason why #[pymethods] has issues is because &self screws up the 'static requirement for into_coroutine. I don't think any async method that accesses member variables can be reasonably supported with the async fn syntax because of the 'static lifetime requirement, so it would have to fall back to impl Future to separate accessing member variables from the async { } body in nearly all cases. I think this is just an unfortunate consequence of how Rust desugars async fn. Idk if this is something that Rust could improve over time, but at the moment I think we're stuck with it. I also can't see the 'static lifetime requirement going away since we can't safely make assumptions about lifetimes across the ffi boundary.

[kngwyu]

What we need for making #[pyfunction] async fn work is just to convert the returning value by into_coroutine, right?
Then I'm not convinced why we need this syntax sugar.

[awestlake87]

Correct, that's all that's needed. There were some conversations about adding it in this thread in the pyo3-asyncio repo, so I figured I'd just open a PR to demonstrate my ideas on it (I probably should have mentioned that in the PR). I'm not pushing for it to be added for the v0.13.2 release or anytime soon either.

Given the caveats with #[pymethods] and the fact that you'll need to specify the runtime in the #[pyfunction] attribute, I think it has some issues. It may just be better not to support it like you said @kngwyu.

[davidhewitt]

Thanks very much for this @awestlake87.

In the long term I would really like to see this syntax supported. I think that one of the great things about #[pymethods] is that you can use them as normal Rust functions also, so it seems natural to extend this to async fn.

At the moment I agree with @kngwyu that we don't urgently need this syntax (as it is just sugar) so I'd prefer to give us more time to learn about the async integration before we think about merging. Especially with the complication around runtimes, as you noted.

[awestlake87]

I was thinking about the problems with #[pymethods] a little bit and I think there might potentially be a solution that would let us use the async fn syntax without using impl Future. If self had a receiver type like self: Py<PyCell<Self>> instead of &self, then that could obey the 'static lifetime requirement. I don't think even the nightly arbitrary_self_types feature would support this just yet since I think Deref is required by the current design (although that's currently up for discussion in the tracker)

[davidhewitt]

I was thinking similarly. I think the receiver would be Py<Self>. We already support &PyCell<Self> which also does not implement Deref<Target = Self>, so I think this would be acceptable to add.

[ThibaultLemaire]

Regarding async #[pymethods],

I'm not too familiar with macros, but I reckon we could support async methods if they only use &self and if Self: Clone.

#[pyclass]
#[derive(Clone)]
struct MyClass {
    // <snip>
}

#[pymethods]
impl MyClass {
    async fn my_function(&self, my_param: u32) -> Vec<String> {
        // <snip>
    }
}

Although I'm not sure how useful that would be in the wild.

Here, for example, I wrote an associated function that behaves just like an async method for Python. It returns the list of book titles written by George Orwell in a hypothetical MongoDB database.

It works because mongodb::Collection is Clone as it uses an Arc under the hood.

[davidhewitt]

I think rather than requiring self: Clone it'd be much better to take Py<Self>, which implements Clone and amounts to just increasing the reference count rather than copying the whole Rust struct.

Thank you for that example 👍

[ThibaultLemaire]

Py<Self> is the very first thing I tried, except I always ended up with some variation of the GIL being used across an await boundary.

error: future cannot be sent between threads safely
   --> src/lib.rs:298:9
    |
95  |     fn new(future: impl Future<Output = PyResult<impl ToPyObject>> + Send + 'static) -> Self {
    |                                                                      ---- required by this bound in `MyFuture::new`
...
298 |         MyFuture::new(async move {
    |         ^^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: within `impl futures::Future`, the trait `std::marker::Send` is not implemented for `Rc<()>`
note: future is not `Send` as this value is used across an await
   --> src/lib.rs:302:19
    |
299 |             let gil = Python::acquire_gil();
    |                 --- has type `GILGuard` which is not `Send`
...
302 |             match future.await {
    |                   ^^^^^^^^^^^^ await occurs here, with `gil` maybe used later
...
308 |         })
    |         - `gil` is later dropped here

In the end, the only way to circumvent that, is to own self (or some part of it) which ended up in not so "sugary" code IMO. (At least with my approach.)

But you are right, strictly speaking, I don't see anything preventing an async fn _(_: Py<Self>) method. I just think that it would be pushing the complexity on the implementer, and I wanted to explore additional reasons for #[pymethods] support for async fn.

[davidhewitt]

Hmm this is probably another motivation for #1068 - where if a Py<Self> only gave out immutable access, it should be able to read the underlying Rust value without the GIL held.

[awestlake87]

I used to run into this compiler error a lot when writing Rust/Python async code, but I think the compiler is being sensible here. Holding the GIL across an await boundary isn't something you typically want to do. If a task is suspended while holding the GIL, then all other async tasks that use the Python GIL will be locked until that task wakes up from its await.

Usually what I do to get around this is wrap all Python calls in a Python::with_gil and store values in PyObject before awaiting. It's a bit tedious I know, but it's worked well for me so far.

I agree that it does push some complexity on the implementer to use Py<Self> in #[pymethods], but I think juggling the GIL between await boundaries is unavoidable. There might be some syntactic sugar that we can add to the macros to make this more seamless, but I'm worried it might involve more macro magic than I'd be comfortable with.

[aviramha]

Should we maybe close this as it's out of scope right now?

[awestlake87]

Sure, no worries on my part. I've been waiting to let the pyo3-asyncio mature before I take another look at the proc macro stuff. I think I can manage to make a downstream solution for this as well instead of making changes to the core!

I can always find it later if need be.

[davidhewitt]

👍 thanks again @awestlake87 for all the work you put into async pyo3!