- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
gh-129195: use future_add_to_awaited_by/future_discard_from_awaited_by in asyncio.staggered.staggered_race #129253
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
gh-129195: use future_add_to_awaited_by/future_discard_from_awaited_by in asyncio.staggered.staggered_race #129253
Conversation
…ited_by in staggered race
ddeeee9    to
    7626ae4      
    Compare
  
    | """ | ||
| # TODO: when we have aiter() and anext(), allow async iterables in coro_fns. | ||
| loop = loop or events.get_running_loop() | ||
| parent_task = tasks.current_task(loop) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if we're not running in a task? Should we assert that this is non-None?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one possible outcome is that there's no task or running loop:
import asyncio.staggered
async def main():
    async def asyncfn():
        pass
    await asyncio.staggered.staggered_race([asyncfn, asyncfn], delay=None)
main().__await__().send(None)we get a nice traceback, saying that:
./python demo.py
Traceback (most recent call last):
  File "/home/graingert/projects/cpython/demo.py", line 9, in <module>
    main().__await__().send(None)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/home/graingert/projects/cpython/demo.py", line 7, in main
    await asyncio.staggered.staggered_race([asyncfn, asyncfn], delay=None)
  File "/home/graingert/projects/cpython/Lib/asyncio/staggered.py", line 66, in staggered_race
    loop = loop or events.get_running_loop()
                   ~~~~~~~~~~~~~~~~~~~~~~~^^
RuntimeError: no running event loopThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you manually pass a loop, and step the coroutine outside of a task:
import asyncio.staggered
async def main(loop):
    async def asyncfn():
        print("hello")
    await asyncio.staggered.staggered_race([asyncfn, asyncfn], delay=None, loop=loop)
    return "world"
loop = asyncio.EventLoop()
coro = main(loop).__await__()
f = coro.send(None)
loop.run_until_complete(f)
try:
    coro.send(None)
except StopIteration as e:
    print(e.value)by some miracle it all works, this is because future_add_to_awaited_by and future_discard_from_awaited_by is noop if any arg is not a future (eg is None) and we don't have any further use of the parent task
./python demo.py
hello
worldso we should probably leave it like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's worth testing this usecase though
| t.cancel() | ||
|  | ||
| propagate_cancellation_error = None | ||
| try: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above this--for cancelling all the coroutines, future_discard_from_awaited_by should get called, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah add_done_callback callbacks are called on cancellation, so it will get called
        
          
                Misc/NEWS.d/next/Library/2025-01-24-10-48-32.gh-issue-129195.89d5NU.rst
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …9d5NU.rst Co-authored-by: Kumar Aditya <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well.
Uh oh!
There was an error while loading. Please reload this page.