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

Coroutines can be started without being awaited #51

Open
RobertCraigie opened this issue Oct 18, 2021 · 2 comments
Open

Coroutines can be started without being awaited #51

RobertCraigie opened this issue Oct 18, 2021 · 2 comments

Comments

@RobertCraigie
Copy link

🐛 Bug Reports

Using the tokio example from the README, the coroutine can be started without awaiting it in python.

I apologise if this is documented somewhere or if this has an obvious solution in rust, I am very new to rust.

🌍 Environment

  • Your operating system and version: Windows
  • Your python version: 3.9
  • How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?: Python.org installer, using a virtualenv
  • Your Rust version (rustc --version): rustc 1.55.0 (c8dfcfe04 2021-09-06)
  • Your PyO3 version: 0.14.5
  • Have you tried using latest PyO3 master (replace version = "0.x.y" with git = "https://github.com/awestlake87/pyo3-asyncio")?: Yes, with the same results

💥 Reproducing

use pyo3::{prelude::*, wrap_pyfunction};

#[pyfunction]
fn rust_sleep(py: Python) -> PyResult<&PyAny> {
    pyo3_asyncio::tokio::future_into_py(py, async {
        tokio::time::sleep(std::time::Duration::from_secs(1)).await;
        println!("hello from rust!");
        Ok(Python::with_gil(|py| py.None()))
    })
}

#[pymodule]
fn my_async_module(py: Python, m: &PyModule) -> PyResult<()> {
    m.add_function(wrap_pyfunction!(rust_sleep, m)?)?;
    Ok(())
}
from my_async_module import rust_sleep
import asyncio

async def main():
    rust_sleep()
    await asyncio.sleep(1)

asyncio.run(main())

Outputs:

hello from rust!

It should be noted that if the await asyncio.sleep(1) is removed, nothing is output, this is different behaviour than standard python coroutines which would output a warning:

RuntimeWarning: coroutine 'sleep' was never awaited
  asyncio.sleep(1)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Built using maturin.

@RobertCraigie RobertCraigie changed the title Coroutines are ran without being awaited Coroutines can be started without being awaited Oct 18, 2021
@awestlake87
Copy link
Owner

awestlake87 commented Oct 18, 2021

This has been brought up before, and I provided a workaround for it in that thread. At the time I dismissed it as a caveat for the library, but since multiple people have mentioned it as unexpected behaviour, I'm thinking it needs to be either clearly documented or potentially fixed in some fashion.

Why the problem exists:

Calling pyo3_asyncio::tokio::future_into_py spawns the Rust future on tokio and signals a Python asyncio.Future when it completes. The await keyword in Python currently provides no interaction aside from waiting for the tokio task to join, and spawning the future happens immediately when pyo3_asyncio::tokio::future_into_py is called.

A simple workaround:

The problem can be avoided by wrapping the futures written in pyo3_asyncio with a simple Python coroutine:

from my_async_module import rust_sleep

async def py_sleep():
    return await rust_sleep()

The coroutine will do nothing when called. Instead, awaiting it the first time will call rust_sleep().

Potential Solutions:

One obvious solution would be to incorporate the workaround into the future_into_py call somehow. This seems doable.

Instead of incorporating it directly into the future_into_py call, we could provide a deferred function that provides this wrapper only when people need / want it.

Alternatively we could delay the call to tokio::spawn until the first time it's awaited (possibly using the __await__ magic function). It's questionable whether or not this will work since we would likely need to involve deriving from asyncio.Future. Deriving from Python classes in PyO3 was not quite there yet the last time I checked. This might involve writing our own future object instead of using asyncio.Future.

Problems with this solution

Fixing this will only fix the first await. After the first await, the Rust future is getting polled until it is completed or cancelled regardless of whether Python is currently awaiting it. This is because we don't have any communication that prevents tokio from polling the future only when Python wants to poll it, only communication to signal completion or cancellation. This is one of the reasons why I have been hesitant to fix this problem.

On the first public release, we called this conversion into_coroutine, but it was later changed to future_into_py because into_coroutine is inaccurate. It's really returning an asyncio.Future, and while it's true that coroutines don't do any work until they are polled, I'm not sure that's true for all Future objects.

I'm a bit split on this issue, so I'd love to hear peoples thoughts on it. I think it's possible to fix, but the question I have is whether it should be documented, fully fixed, or have the workaround as a separate library function.

@RobertCraigie
Copy link
Author

Ah I hadn't considered that workaround, thank you.

The solution that makes the most sense to me is incorporating the workaround into the future_into_py call however I don't really have the knowledge to comment any further.

Thanks for the incredibly detailed and informative reply!

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

No branches or pull requests

2 participants