-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Changes from 4 commits
9bb80fa
7626ae4
c8e7412
21ab6f5
f3044a2
d6ed8e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
from . import exceptions as exceptions_mod | ||
from . import locks | ||
from . import tasks | ||
from . import futures | ||
|
||
|
||
async def staggered_race(coro_fns, delay, *, loop=None): | ||
|
@@ -63,6 +64,7 @@ async def staggered_race(coro_fns, delay, *, loop=None): | |
""" | ||
# 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) | ||
enum_coro_fns = enumerate(coro_fns) | ||
winner_result = None | ||
winner_index = None | ||
|
@@ -73,6 +75,7 @@ async def staggered_race(coro_fns, delay, *, loop=None): | |
|
||
def task_done(task): | ||
running_tasks.discard(task) | ||
futures.future_discard_from_awaited_by(task, parent_task) | ||
if ( | ||
on_completed_fut is not None | ||
and not on_completed_fut.done() | ||
|
@@ -110,6 +113,7 @@ async def run_one_coro(ok_to_start, previous_failed) -> None: | |
this_failed = locks.Event() | ||
next_ok_to_start = locks.Event() | ||
next_task = loop.create_task(run_one_coro(next_ok_to_start, this_failed)) | ||
futures.future_add_to_awaited_by(next_task, parent_task) | ||
running_tasks.add(next_task) | ||
next_task.add_done_callback(task_done) | ||
# next_task has been appended to running_tasks so next_task is ok to | ||
|
@@ -148,6 +152,7 @@ async def run_one_coro(ok_to_start, previous_failed) -> None: | |
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above this--for cancelling all the coroutines, There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
ok_to_start = locks.Event() | ||
first_task = loop.create_task(run_one_coro(ok_to_start, None)) | ||
futures.future_add_to_awaited_by(first_task, parent_task) | ||
running_tasks.add(first_task) | ||
first_task.add_done_callback(task_done) | ||
# first_task has been appended to running_tasks so first_task is ok to start. | ||
|
@@ -171,4 +176,4 @@ async def run_one_coro(ok_to_start, previous_failed) -> None: | |
raise propagate_cancellation_error | ||
return winner_result, winner_index, exceptions | ||
finally: | ||
del exceptions, propagate_cancellation_error, unhandled_exceptions | ||
del exceptions, propagate_cancellation_error, unhandled_exceptions, parent_task |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Support reporting call graph information from :func:`!asyncio.staggered.staggered_race` | ||
graingert marked this conversation as resolved.
Show resolved
Hide resolved
|
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:
we get a nice traceback, saying 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.
if you manually pass a loop, and step the coroutine outside of a task:
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
so 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