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

Async pymethod with receiver can not pass any other arguments #4005

Closed
reswqa opened this issue Mar 28, 2024 · 2 comments · Fixed by #4015
Closed

Async pymethod with receiver can not pass any other arguments #4005

reswqa opened this issue Mar 28, 2024 · 2 comments · Fixed by #4015
Labels

Comments

@reswqa
Copy link
Contributor

reswqa commented Mar 28, 2024

Bug Description

When I tried to introduce argument other than receiver(i.e. &self/&mut self) to async method, I found that the code(see Steps to Reproduce) would not compile(see the Backtrace part), even though the parameter was actually Send + 'static itself.

I'm not sure if this is by design, but it looks strange to me.


I'm new to the PyO3's code base, but I took a closer look.

FnType::Fn(SelfType::Receiver { mutable: false, .. }) => {
         quote! {{
           let __guard = #pyo3_path::impl_::coroutine::RefGuard::<#cls>::new(&#pyo3_path::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
           async move { function(&__guard, #(#args),*).await }
        }}
}

The expanded code for the async block is:

async move { function(&__guard, ::pyo3::impl_::deprecations::inspect_type(::pyo3::impl_::extract_argument::extract_argument(&::pyo3::impl_::extract_argument::unwrap_required_argument(output[0usize]), &mut holder_0, "v")?, &gil_refs_checker_0)).await }

This move the output(Option<pyo3::Borrowed<', ', pyo3::PyAny>> here) to async block and therefore does not satisfy the requirements of Send.

I think we should evaluate the arg before move it, like:

let arg = ::pyo3::impl_::deprecations::inspect_type(::pyo3::impl_::extract_argument::extract_argument(&::pyo3::impl_::extract_argument::unwrap_required_argument(output[0usize]), &mut holder_0, "a")?, &gil_refs_checker_0)
async move { function(&__guard, arg).await }

If this idea make sense, I can come up with a PR then.

Steps to Reproduce

#[test]
fn test_async_receiver_with_args() {
    #[pyclass]
    struct Foo;
    #[pymethods]
    impl Foo {
        async fn bar(&self, v: i32) -> PyResult<()>{
            // do nothing
            Ok(())
        }
    }
}

Backtrace

317 |     #[pymethods]
    |     ^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: within `{async block@tests/test_foo.rs:317:5: 317:17}`, the trait `std::marker::Send` is not implemented for `NonNull<pyo3::ffi::PyObject>`
note: captured value is not `Send`
   --> tests/test_foo.rs:317:5
    |
317 |     #[pymethods]
    |     ^^^^^^^^^^^^ has type `[Option<pyo3::Borrowed<'_, '_, pyo3::PyAny>>; 1]` which is not `Send`
note: required by a bound in `new_coroutine`
   --> xxx/pyo3/src/impl_/coroutine.rs:22:40
    |
15  | pub fn new_coroutine<F, T, E>(
    |        ------------- required by a bound in this function
...
22  |     F: Future<Output = Result<T, E>> + Send + 'static,
    |                                        ^^^^ required by this bound in `new_coroutine`
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

Your operating system and version

MacOs 14.3.1

Your Python version (python --version)

3.11

Your Rust version (rustc --version)

rustc 1.76.0 (07dca489a 2024-02-04)

Your PyO3 version

0.21.0

How did you install python? Did you use a virtualenv?

By virtualenv

Additional Info

No response

@reswqa reswqa added the bug label Mar 28, 2024
@davidhewitt
Copy link
Member

Hi, thanks for the report. Your analysis that this is a bug, and the proposed fix, looks correct to me! A PR would be greatly appreciated 🙏

@reswqa
Copy link
Contributor Author

reswqa commented Mar 29, 2024

Thank you for your confirmation. I will come up with a fix recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants