Skip to content

Detect asyncio when the loop is running without a Task - #59

Closed
gyanu2507 wants to merge 3 commits into
python-trio:masterfrom
gyanu2507:asyncio-running-loop
Closed

Detect asyncio when the loop is running without a Task#59
gyanu2507 wants to merge 3 commits into
python-trio:masterfrom
gyanu2507:asyncio-running-loop

Conversation

@gyanu2507

Copy link
Copy Markdown

current_task() is None for Twisted's asyncioreactor and for call_soon callbacks, so sniffio treated that as "not in async context" even though asyncio.get_running_loop() succeeds.

If there is no current Task, fall back to get_running_loop() before giving up.

Fixes #51

Twisted's asyncioreactor (and call_soon callbacks) run on the asyncio loop
without an asyncio.Task, so current_task() is None and sniffio raised
AsyncLibraryNotFoundError.

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the regression test fail promptly when the fallback is absent. In sync_cb(), Python evaluates current_async_library() before loop.stop(). On the exact base, that call raises AsyncLibraryNotFoundError; asyncio reports the callback exception, skips loop.stop(), and run_forever() then waits indefinitely. I reproduced this event-loop behavior with a stdlib-only subprocess: the callback ran, but the process did not terminate. Put loop.stop() in a finally block so the assertion outside the callback produces a deterministic failure on the red path.

Please also add newsfragments/51.bugfix.rst. CONTRIBUTING.md delegates to the Trio contribution guide, which says every pull request with a user-visible effect should add a newsfragment; this is the user-visible bugfix for #51.

I verified this against exact head 0cbbda24fb2919c21e803580c4c5fe447bab3fed and current merge a9b862f6d73b6afefc39704de56efcf22c8c8df1. The production fallback itself is reasonable, and the live CI is fully green: 20/20 check runs, 1/1 check suite, 1/1 Actions workflow, and the Read the Docs status.

Disclosure: this review was prepared with Codex assistance; I independently verified the exact refs, source path, test behavior, repository policy, interactions, and live CI before submission.

@gyanu2507

Copy link
Copy Markdown
Author

Updated: loop.stop() is in a finally so the test still exits if the fallback is missing, and newsfragments/51.bugfix.rst is in.

@gyanu2507
gyanu2507 requested a review from fallenmi September 1, 2026 05:55

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two requested changes are present on exact head 98a876b4f8a28a168a61172b5fd81c65e7b1f114. loop.stop() now runs in a finally, so a detection failure still exits run_forever() and the outer assertion fails deterministically; newsfragments/51.bugfix.rst is also present.

I rechecked the full production path and current merge: thread-local/context-variable precedence and existing Task detection remain intact, the fallback only recognizes a running asyncio loop without a Task, and all three touched blobs are identical between head and merge. Live CI is fully green: 20/20 checks, the suite, workflow, and Read the Docs status. I found no remaining blocker.

Disclosure: I used OpenAI Codex and Claude Sonnet to assist this rereview; I verified the exact refs, old-to-new delta, failure-path behavior, production semantics, repository policy and interactions, merge blobs, and live CI before submission.

@x42005e1f

x42005e1f commented Sep 1, 2026

Copy link
Copy Markdown

This is a duplicate of #39, and with a worse implementation: there is no point in trying current_task() before get_running_loop() when the former will already fail outside the event loop, and the latter fully covers all cases handled by the former, so the latter is sufficient on its own. Moreover, get_running_loop() is faster than current_task(), so… what is the point of preferring the latter and adding the former as a fallback?

CPython 3.10.21 (asyncio REPL):

>>> import asyncio
>>> from timeit import timeit
>>> timeit(asyncio.current_task)
0.6053914040094241
>>> timeit(asyncio.get_running_loop)
0.46340504300314933
>>> import sniffio    # python-trio/sniffio
>>> import sniffio39  # python-trio/sniffio#39
>>> import sniffio59  # python-trio/sniffio#59
>>> timeit(sniffio.current_async_library)
Traceback (most recent call last):
  File "/home/user/.local/share/mise/installs/python/3.10.21/lib/python3.10/concurrent/futures/_base.py", line 458, in result
    return self.__get_result()
  File "/home/user/.local/share/mise/installs/python/3.10.21/lib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
    raise self._exception
  File "/home/user/.local/share/mise/installs/python/3.10.21/lib/python3.10/asyncio/__main__.py", line 34, in callback
    coro = func()
  File "<console>", line 1, in <module>
  File "/home/user/.local/share/mise/installs/python/3.10.21/lib/python3.10/timeit.py", line 234, in timeit
    return Timer(stmt, setup, timer, globals).timeit(number)
  File "/home/user/.local/share/mise/installs/python/3.10.21/lib/python3.10/timeit.py", line 178, in timeit
    timing = self.inner(it, self.timer)
  File "<timeit-src>", line 6, in inner
  File "/home/user/.local/lib/python3.10/site-packages/sniffio/_impl.py", line 93, in current_async_library
    raise AsyncLibraryNotFoundError(
sniffio._impl.AsyncLibraryNotFoundError: unknown async library, or not in async context
>>> timeit(sniffio39.current_async_library)
0.8540417910553515
>>> timeit(sniffio59.current_async_library)
1.519261592067778

current_task() already raises outside a loop, so probing it first is
strictly slower and still needs the get_running_loop fallback for
call_soon callbacks.
@gyanu2507

Copy link
Copy Markdown
Author

You're right — get_running_loop() already covers the Task case, so the current_task() probe was just extra work. Dropped it; detection is get_running_loop() only now.

This PR is still the narrow bugfix with the call_soon test. #39 is the broader docs change on top of _get_running_loop().

@x42005e1f

x42005e1f commented Sep 1, 2026

Copy link
Copy Markdown

#39 is the broader docs change on top of _get_running_loop().

See #38. It is about semantics. Your PR changes that very same semantics. And #39 addresses the very same issue you describe at the beginning of this PR (#35).


To clarify, current_task()get_running_loop() is more of a change than a fix, since it affects behavior outside of tasks. For example, what if a user expects current_async_library() to succeed only in async tasks (async functions), and uses this expectation to determine whether to "return a coroutine object or run as a regular function"? In my opinion, this is one of the reasons why #38/#39 is worded so formally.

See #35 (comment).


And as you can see, the documentation refers to coroutines, which would no longer correspond to the changed semantics.

@x42005e1f

Copy link
Copy Markdown

However, it is worth noting that AnyIO and Trio already have semantics that differ from those described in the sniffio documentation:

>>> import sniffio
>>> def test():
...     try:
...         print(sniffio.current_async_library())
...     except sniffio.AsyncLibraryNotFoundError:
...         print("unknown")
>>> import asyncio
>>> async def asyncio_test():
...     asyncio.get_running_loop().call_soon(test)
>>> import trio
>>> async def trio_test():
...     trio.lowlevel.current_trio_token().run_sync_soon(test)
>>> import anyio
>>> asyncio.run(asyncio_test())
unknown
>>> anyio.run(asyncio_test)
asyncio
>>> trio.run(trio_test)
trio

On the one hand, one could argue that the documentation refers to coroutines only in the context of adding support for a new library, and says nothing about the behavior of current_async_library() outside of a coroutine. On the other hand, what exactly is a "synchronous context" in relation to AsyncLibraryNotFoundError? So the problem is ambiguous.

@gyanu2507

Copy link
Copy Markdown
Author

You're right that this changes the documented "must be inside a Task" reading — that's the bug. get_running_loop() is the whole check now.

#39 can still land the docs/_get_running_loop() wording on top of this; the call_soon test here is the part that issue didn't have. The AnyIO/Trio examples are a separate docs problem (they already disagree with the current text) and I would not fold them into this PR.

@A5rocks

A5rocks commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Oh please shut up Claude.

@x42005e1f interesting! idk what #39 needs from now but I think it's a good idea, especially if as you note the definition of what is "running" is already different between Trio and asyncio.

@A5rocks A5rocks closed this Sep 2, 2026
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 this pull request may close these issues.

sniffio fails to detect asyncio when run via twisted internet asyncio reactor

4 participants