dabeaz / curio

Good Curio!
Other
4.03k stars 241 forks source link

Curio can't run async functions defined in cython #326

Closed stharding closed 4 years ago

stharding commented 4 years ago

Check out this ipython session:

In [1]: import curio                                                                                                                                                                                                 

In [2]: %load_ext cython                                                                                                                                                                                             

In [3]: async def foo(): 
   ...:     print("async, from python") 
   ...:                                                                                                                                                                                                              

In [4]: curio.run(foo)                                                                                                                                                                                               
async, from python

In [5]: %%cython 
   ...: async def bar(): 
   ...:     print("async, from cython") 
   ...:                                                                                                                                                                                                              

In [6]: curio.run(bar)                                                                                                                                                                                               
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-5468b93f18dd> in <module>
----> 1 curio.run(bar)

/usr/local/lib/python3.8/site-packages/curio/kernel.py in run(corofunc, with_monitor, selector, debug, activations, *args, **kernel_extra)
    821 
    822     with kernel:
--> 823         return kernel.run(corofunc, *args)
    824 
    825 # An Activation is used to monitor and effect what happens

/usr/local/lib/python3.8/site-packages/curio/kernel.py in run(self, corofunc, shutdown, *args)
    135             raise RuntimeError("Can't run a kernel that's been shut down or crashed. Create a new kernel.")
    136 
--> 137         coro = meta.instantiate_coroutine(corofunc, *args) if corofunc else None
    138 
    139         with meta.running():

/usr/local/lib/python3.8/site-packages/curio/meta.py in instantiate_coroutine(corofunc, *args, **kwargs)
    102         coro = corofunc(*args, **kwargs)
    103         if not inspect.iscoroutine(coro):
--> 104             raise TypeError(f'Could not create coroutine from {corofunc}')
    105         return coro
    106 

TypeError: Could not create coroutine from <built-in function bar>

Looks like trio had a similar problem that was resolved by wrapping 'built-in' async functions in an async python wrapper.

dabeaz commented 4 years ago

Is there any way for Curio to know that bar is some kind of "built-in" async function?

njsmith commented 4 years ago

I think you want https://docs.python.org/3/library/collections.abc.html#collections.abc.Coroutine

On Sat, Jun 13, 2020, 18:05 David Beazley notifications@github.com wrote:

Is there any way for Curio to know that bar is some kind of "built-in" async function?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dabeaz/curio/issues/326#issuecomment-643702841, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEU42FZFR36EKEOILOJH73RWQO57ANCNFSM4N5G6GTQ .

dabeaz commented 4 years ago

I won't even consider looking at this unless a Cython async function can pass the equivalent of this test involving asyncio and one specific feature of curio:

import curio.meta
import asyncio

async def test():
    assert curio.meta.from_coroutine(1)

asyncio.run(test())

Also, maybe the inspect module should be fixed.

dabeaz commented 4 years ago

As a followup note: the main concern here is being able to test if code is being called from an asynchronous environment or not. Many parts of curio rely on this--and it's just not going to work if it can't be determined for Cython async functions. I do have a thought on this I'm going to investigate further though.

stharding commented 4 years ago

Thanks Dave, understood. I'll take a look and see if I can come up with something as well.

stharding commented 4 years ago

I think you want https://docs.python.org/3/library/collections.abc.html#collections.abc.Coroutine

@njsmith isinstance(bar(), abc.Coroutine) will return True, but I think we need a way to detect that bar is a coroutine function rather than detect that bar() is a coroutine.

dabeaz commented 4 years ago

If bar() is something that returns a coroutine (and the result can be detected as such by the inspect module), using something like curio.run(bar()) might work now.

I'm still investigating the curio.meta.from_coroutine() matter. I don't really know what Cython does for async functions or how they manifest in the environment, but there might be an alternative way to achieve the same effect.

stharding commented 4 years ago

Odd. Looks like the inspect module can't tell, but the collections.abc module can ...

In [1]: %load_ext cython                                                                                                                                                                       

In [2]: %%cython 
...: async def bar(): 
...:     print("async, from cython") 
...:                                                                                                                                                                                        

In [3]: import inspect                                                                                                                                                                         

In [4]: inspect.iscoroutine(bar())                                                                                                                                                             
/usr/local/bin/ipython:1: RuntimeWarning: coroutine 'bar' was never awaited
#!/usr/local/bin/python
Out[4]: False

In [5]: from collections import abc                                                                                                                                                            

In [6]: isinstance(bar(), abc.Coroutine)                                                                                                                                                       
/usr/local/bin/ipython:1: RuntimeWarning: coroutine 'bar' was never awaited
#!/usr/local/bin/python
Out[6]: True
stharding commented 4 years ago

A related open issue in the Cython project: https://github.com/cython/cython/issues/2273

dabeaz commented 4 years ago

I have modified Curio to use collections.abc.Coroutine for checking. This seems to fix the immediate issue with cython.

I have investigated the from_coroutine() behavior with Cython and it seems to "work", but perhaps only so by accident. The Cython async function identifies itself as a "builtin function". As such, it doesn't create a normal stack frame. So, instead, from_coroutine() picks up information from the closest python-level async function that calls into Cython. By necessity, this has to be an async function--since there's no other way to await for the result of the Cython function.

I'm going to leave it for now, but might revisit this later.

stharding commented 4 years ago

Thanks Dave, this does address the problem. It would probably be better if Cython made async def functions detectable via iscoroutinefunction.

dabeaz commented 4 years ago

I do agree that it would be useful for Cython async functions to identify themselves as such. Independently of Curio, there are other situations where it is useful to know that information. For example, knowing that a function produces a coroutine might be of interest if writing a decorator function.

The whole from_coroutine() thing I mention is more Curio specific. I can probably work with what I've got for now---no changes to Cython.

stharding commented 4 years ago

Yeah, makes sense. I looked at your commits this morning and tried out the master branch locally and everything seems to work as advertised. I'm happy to call this issue closed unless there is more you wanted to do.