asyncio.iscoroutinefunction returns false for Cython async function objects

[danigosa]

Referring to #2092

@scoder the implementation in asyncio is ridiculous

def iscoroutinefunction(func):
    """Return True if func is a decorated coroutine function."""
    return (inspect.iscoroutinefunction(func) or
            getattr(func, '_is_coroutine', None) is _is_coroutine)

So once we agreed inspect returns False, it happens to be that CPython just add a simple attribute to the function _is_coroutine, I think if Cython does this adding up this attribute it will make everything work

Here: https://github.com/python/cpython/blob/789e359f51d2b27bea01b8c6c3bf090aaedf8839/Lib/asyncio/coroutines.py#L152

And here:
https://github.com/python/cpython/blob/789e359f51d2b27bea01b8c6c3bf090aaedf8839/Lib/asyncio/coroutines.py#L160

So the solution as it can't be another it results in this, from my test in iPython:

In [46]: fut = init_connection()
In [47]: asyncio.iscoroutine(fut)
Out[47]: True

In [49]: init_connection.__class__
Out[49]: cython_function_or_method

In [50]: asyncio.iscoroutinefunction(init_connection)
Out[50]: False

In [56]: from asyncio.coroutines import _is_coroutine

In [57]: init_connection._is_coroutine = _is_coroutine

In [58]: asyncio.iscoroutinefunction(init_connection)
Out[58]: True

Dirty but easy. Don't know if this is a 5 seconds issue or a hara-kiri, theoretically it should be easy to plug the attribute runtime from from asyncio.coroutines import _is_coroutine, or not.

This is a super annoying problem (I don't blame anyone with the word bug) that make cythonized code to be very dramatic in several asyncio frameworks. asyncio.iscoroutine work well with cythonized funcs for the silliest reason I think.

Following the cyfunctions above, create_token is NOT a coroutine, while init_connection is an async function, this workaround works in both so it's ok if you can schedule the functions whether are coros or not, no way to easily fix this with asyncio.iscoroutinefunction or at least easier than that:

In [59]: async def test():
    ...:     await create_token({})
    ...:
In [61]: asyncio.get_event_loop().run_until_complete(test())
TypeError: object str can't be used in 'await' expression

In [73]: async def test():
    ...:     return await init_connection()
Out[74]: <asyncpg.pool.Pool at 0x7f78e04752c8>

Here comes the magic:

In [69]: async def test():
    ...:     return await asyncio.coroutine(create_token)({})
In [70]: asyncio.get_event_loop().run_until_complete(test())
Out[70]: 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzUxMiJ9.eyJleHAiOjE1MjU1NTk0NTB9.v8wRZGIke6RYizIpJhl4oaypKyvuVARKaiq0KdpJg6XJ0qTB0o76BuTLera6kSQ_5qXnAb7_9DQSadwdRqgPmw'

In [71]: async def test():
    ...:     return await asyncio.coroutine(init_connection)()
In [72]: asyncio.get_event_loop().run_until_complete(test())
Out[72]: <asyncpg.pool.Pool at 0x7f78df48af48>

With that the workaround:

def nasty_iscoroutinefunction(func):
      is_coro = asyncio. iscoroutinefunction(func)
      if func.__class__.__name__ == 'cython_function_or_method':
            # It's cythonized, danger!
            is_coro = True  # We'll make it possible
            func = asyncio.coroutine(func)
      return is_coro

This works!

[danigosa]

Found an interesting, better and working workaround:

https://github.com/timothycrosley/hug/blob/develop/hug/introspect.py#L33

def is_coroutinefunction(function):
    """Returns True if the passed in function is a coroutine"""
    return function.__code__.co_flags & 0x0080 or getattr(function, '_is_coroutine', False)

[pohmelie]

Is there plans to fix this?
@danigosa, AFAIR you suggest to replace asyncio.iscoroutinefunction in asyncio module, but I think this should be fixed on cython side instead, so the original asyncio method will work as expected.

[pohmelie]

Hm, it looks like tests with code similar to inspect.py are disabled.

[scoder]

The problem is that if we make Cython coroutines look too much like Python coroutines at the C level (e,g, by setting the CO_COROUTINE flag on them), then CPython might start doing things with them that it shouldn't. Even if we find that it doesn't do that right now, it might change at any time, and then people have their Cython modules out there that break. So this is risky business.

Similarly, unconditionally patching inspect might have unfortunate side effects as well, even if requires an opt-in. It's impossible to guess what mixture of code end users will have on their systems.

[pohmelie]

@scoder
I'm half way to make PR to add CO_COROUTINE to coroutine wrappers. But if it will not be accepted anyway, I will stop.

AFAIR, right now c wrapper looks like:

def c_wrapper(...):
    return coroutine(...)

It looks like this can be changed to

@types.coroutine
def c_wrapper(...):
    return (yield from coroutine(...))

In this case all stuff is on python shoulders.

[pohmelie]

@scoder
I have made some dirty hack and it looks like it works. Any thoughts about such solution?
pohmelie@5a84869

import asyncio


async def foo():
    pass


print("foo", foo.__code__.co_flags)
print("foo", asyncio.iscoroutinefunction(foo))
print(__cython_coroutine_wrapper)

compiled code output:

('foo', 319)
('foo', True)
<function coroutine at 0x7f30b9a0e620>

[pohmelie]

Linked code is not working in all cases, it is just quick proof of concept.

[pohmelie]

By the way, I can't find any problem with add CO_COROUTINE flag, since

async def afoo():
    ...

is same as

def foo():
    return afoo()

you can await foo() and it will be absolutely same as await afoo().

[scoder]

There is at least this code in CPython, which I'd rather not want to trigger for Cython coroutines:
https://github.com/python/cpython/blob/caba55b3b735405b280273f7d99866a046c18281/Python/ceval.c#L3906-L3949
Can't say right now if it's possible to do that from Python code, probably via eval&friends.

Also, what we are discussing here actually applies to these three functions in inspect.py: isgeneratorfunction(), iscoroutinefunction() and isasyncgenfunction().

https://github.com/python/cpython/blob/41254ebd5e4f40a2e095d8aaea60bf3973de4647/Lib/inspect.py#L171-L194

[pohmelie]

But why old-style yield from coroutines with asyncio.coroutine works fine? Why we can't just wrap new-style coroutines with same decorator on building ast step?

[keakon]

@scoder Can you consider adding new flags like CO_CYTHON_COROUTINE?

I'm using Starlette, it requires asyncio.iscoroutinefunction() to distinguish coroutine functions. I can't fix it because Cython hides everything about coroutine functions.

[scoder]

Can't starlette do a type-check against the Coroutine ABC instead?

[keakon]

How can I know a function will return a coroutine without call it?

[scoder]

How can I know a function will return a coroutine without call it?

You cannot. There is no way to do that. It's called the "halting problem".
https://en.wikipedia.org/wiki/Halting_problem

[keakon]

But CPython can:

>>> async def f():
...   pass
...
>>> import inspect
>>> inspect.iscoroutinefunction(f)
True
>>> import asyncio
>>> asyncio.iscoroutinefunction(f)
True

[scoder]

No, it can't:

>>> async def func(): pass
>>> import inspect
>>> inspect.iscoroutinefunction(func)
True
>>> def retfunc(): return func()
>>> inspect.iscoroutinefunction(retfunc)
False
>>> type(func())
<class 'coroutine'>
>>> type(retfunc())
<class 'coroutine'>

[dhiltonp]

Is there currrently a way to test if a cython function or method is async? Without that, we can't programmatically fix it even on a case-by-case basis...

[scoder]

There doesn't have to be anything special about a callable that returns an awaitable. It does not have to be an async function. It can be any callable, even including a class. The idea of guessing the return type of a callable, without actually looking at the type of the object that it returns, seems inherently flawed to me.

IMHO, the work-around is to call the callable and look at the value it returns. If that is something to await, then await it. If not, then not.

However, as a clean fix, I would suggest to always require/pass/return something that is awaitable, and not handle the callables differently based on their type.

[pohmelie]

the work-around is to call the callable and look at the value it returns

I think it is bad. Since this promotes bad practice of callable returns awaitable vs awaitable is callable. This should not work like this.

[dhiltonp]

I don't really care that a regular function may return an awaitable, I care about compatibility (or a possibility of compatibility) with CPython. For CPython, iscoroutinefunction returns true if the function was defined with async. As far as I can tell, Cython does not expose this information in any way.

There may be reasons for object.__code__.co_flags & CO_COROUTINE to not be true for async functions in Cython - but at a minimum there must be a programmatic way to determine that a function was defined with async.

[scoder]

I'll accept a PR that adds an _is_coroutine flag to CyFunction and sets it for async functions.

There are already tests in test_coroutine_pep492.pyx that have their call to inspect.iscoroutinefunction() commented out. Note that this file is copied from CPython, so don't change it too much. Just enable those lines for the CPython versions (sys.version_info) that support them.

EDIT: sorry, we are aiming for asyncio.iscoroutinefunction() in this ticket, not inspect.iscoroutinefunction(). The test to extend is tests/run/asyncio_generators.srctree.

[scoder]

@pohmelie "callable returns awaitable vs awaitable is callable" – note that for async functions, the callable is the function, and the awaitable is the return value of the call. An async function (object) is not awaitable.

[fornellas]

There doesn't have to be anything special about a callable that returns an awaitable.
(...)
IMHO, the work-around is to call the callable and look at the value it returns. If that is something to await, then await it. If not, then not.

@scoder for good or bad, Python really does not make a lot of distinctions between async / sync functions. When writing tests though, we do mock things that maybe sync / async and calling them to see if they are sync / async is just not an option. Knowing before calling, is extremely useful, as it enables mocks to detect bugs (eg: mocking an async function with a sync function).

This is how I bumped into this bug. I work on TestSlide, which Facebook open sourced last year. The gist of it is that its mocks have significantly more interface validation than vanilla Mock(spec=...). When asyncio support was added recently, iscoroutinefunction was used to validate mocking of sync / async methods, which turned out to be completely broken for Thrift clients, which are all Cython async.

Perhaps having cython.iscoroutinefunction would be simpler to implement, so we can start using right on? I have limited bandwidth at the moment to deeply dig into this, but with some nice pointers to where to look, I might be able to craft a PR for this.

[dhiltonp]

Just for tracking, the corresponding python issue:

https://bugs.python.org/issue38225

And a related cython issue: #3143

[graingert]

The problem is that if we make Cython coroutines look too much like Python coroutines at the C level (e,g, by setting the CO_COROUTINE flag on them), then CPython might start doing things with them that it shouldn't. Even if we find that it doesn't do that right now, it might change at any time, and then people have their Cython modules out there that break. So this is risky business.

inspect.iscoroutinefunction is now documented as using this mechanism to support duck-type async functions in this way
in addition unittest.mock.AsyncMock has always set CO_COROUTINE in self.__code__.co_flags https://github.com/python/cpython/blame/main/Lib/unittest/mock.py#L2175-L2176

[scoder]

The fact that unittest.mock.AyncMock uses a fake code object and sets a flag on it does not mean that the same thing will work for us at the C level with a real code object using the CO_COROUTINE flag. However, this should at least work safely in CPython <= 3.10, probably up to 3.11, and from those versions on, the inspect change in python/cpython#94050 would take over (from the PoV of a user) if we ever need to remove the flag again. So I'd deem it ok to generally enable the flag for coroutines in Cython 3.0. Until further notice. :)

[scoder]

I revived an old change that I made for this back in 2019 as #4902.

[scoder]

… and noticed that it probably doesn't help anyone because inspect ignores it in earlier Python versions. Not sure if that leaves any value in adding the flag.

[graingert]

inspect.iscoroutinefunction ignores it in earlier python versions however asyncio.iscoroutinefunction still calls inspect.iscoroutinefunctionand the old decorator behavior of asyncio.iscoroutinefunction is undocumented and pending deprecation

[gshmu]

encode/starlette#1777 '_cython_3_0_0a10.coroutine' object is not callable

is cython bug?

[da-woods]

encode/starlette#1777 '_cython_3_0_0a10.coroutine' object is not callable

is cython bug?

It's the same issue: that Cython coroutines aren't enough like Python coroutines to pass inspect. That's mainly down to inspect rather than Cython. It's unlikely to be possible to fix on Python <3.11.