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

mis-detects curio as asyncio when running in curio in asyncio.to_thread (py3.9) #20

Open
graingert opened this issue Mar 3, 2021 · 6 comments · May be fixed by #28
Open

mis-detects curio as asyncio when running in curio in asyncio.to_thread (py3.9) #20

graingert opened this issue Mar 3, 2021 · 6 comments · May be fixed by #28

Comments

@graingert
Copy link
Member

graingert commented Mar 3, 2021

import curio
import sniffio
import asyncio

async def current_framework():
    return sniffio.current_async_library()


async def amain():
    sniffio.current_async_library()
    return await asyncio.to_thread(curio.run, current_framework)


print(asyncio.run(amain()))  # prints asyncio - should print curio
@graingert graingert changed the title mis-detects asyncio when running in curio in asyncio.to_thread (py3.9) mis-detects curio as asyncio when running in curio in asyncio.to_thread (py3.9) Mar 3, 2021
@graingert
Copy link
Member Author

graingert commented Mar 4, 2021

this also happens when running curio inside an asyncio coro

import curio
import asyncio
import sniffio


async def run_in_curio():
    return sniffio.current_async_library()

async def run_curio():
    sniffio.current_async_library()
    return curio.run(run_in_curio)

print(asyncio.run(run_curio()))  # prints asyncio - should print curio

@graingert
Copy link
Member Author

and the other way around, running asyncio inside a curio coro:

import curio
import asyncio
import sniffio


async def run_in_asyncio():
    return sniffio.current_async_library()

async def run_asyncio():
    return asyncio.run(run_in_asyncio())

print(curio.run(run_asyncio))  # prints asyncio - should print curio

@njsmith
Copy link
Member

njsmith commented Mar 4, 2021

Calling curio.run inside an asyncio task and calling asyncio.run inside a curio task are probably not fixable? At least I can't think of any way to do it without some explicit assistance from curio+asyncio. I'm not super bothered about that though, because doing this doesn't really make any sense anyway.

The top-level problem with to_thread does seem like a real issue though. It's happening because sniffio uses a ContextVar to track the current loop, so (1) sniffio is detecting asyncio and caching that in the contextvar, (2) asyncio is blindly copying the entire Context into the thread, (3) sniffio in the thread is re-reading its cached contextvar, even though it's no longer appropriate.

As we discussed a bit on gitter, I'm no longer convinced that ContextVars are really the right approach here, for this and other reasons (in particular, the cvar approach originally assumed that trio/asyncio interop would be through trio-asyncio where we emulate the asyncio operations and have a chance to fix up the cvars; but now trio/asyncio via guest mode is also on the table, and I don't see how the cvar approach can work there, because it uses native asyncio ops and there's no chance for us to intercept trio<->asyncio transitions and fixup the cvar.)

Probably we should just use a thread-local instead of a cvar. It'll make sniffio slightly slower on non-cooperating libraries like asyncio/curio, but I don't see any way to avoid that. Something like:

class CurrentLibHolder(threading.local):
    value = None

_current_lib_holder = CurrentLibHolder()

def set_current_lib(new_lib):
    old_lib = _current_lib_holder.value
    def restore():
        _current_lib_holder.value = old_lib
    _current_lib_holder.value = new_lib
    return restore

def get_current_lib():
    lib = _current_lib_holder.value
    if lib is not None:
        return lib
    if asyncio.current_task() is not None:
        return "asyncio"
    ...

@graingert
Copy link
Member Author

Calling curio.run inside an asyncio task and calling asyncio.run inside a curio task are probably not fixable?

You have to walk the stack and see which library is calling g.send

@njsmith
Copy link
Member

njsmith commented Mar 4, 2021

We're not walking the stack just to fix an issue with code that no-one should even be writing in the first place :-)

@graingert
Copy link
Member Author

Calling curio.run inside an asyncio task and calling asyncio.run inside a curio task are probably not fixable?

Turns out it's totally fixable because asyncio and curio both communicate via sys.set_asyncgen_hooks

@graingert graingert linked a pull request Aug 3, 2022 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants