gh-145107: simplify staggered race using eager_start=False#145108
gh-145107: simplify staggered race using eager_start=False#145108graingert wants to merge 5 commits intopython:mainfrom
Conversation
graingert
requested review from
1st1,
asvetlov,
kumaraditya303 and
willingc
as code owners
|
LGTM! |
There was a problem hiding this comment.
Pull request overview
This PR simplifies asyncio.staggered.staggered_race() by removing the internal “ok_to_start” synchronization mechanism and instead creating tasks with eager_start=False to avoid eager-execution ordering issues (gh-145107).
Changes:
- Remove the
ok_to_start/next_ok_to_startEventgating from the internalrun_one_coro()chain. - Create staggered-race tasks with
loop.create_task(..., eager_start=False)to prevent eager execution. - Add a Misc/NEWS.d entry documenting the change.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
Lib/asyncio/staggered.py |
Reworks task creation in staggered_race() to use eager_start=False and drops the extra coordination events. |
Misc/NEWS.d/next/Library/2026-02-22-11-27-57.gh-issue-145107.ZlQXEZ.rst |
Adds a release-note fragment for the staggered_race change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| simplify asyncio's ``staggered_race`` using ``eager_start=False`` | |||
There was a problem hiding this comment.
NEWS entry formatting doesn’t match the usual Misc/NEWS.d fragments (leading space, starts with lowercase, and lacks a period). Please rephrase to a full sentence starting with a capital letter and ending with a period (and keep consistent module naming, e.g. “asyncio.staggered_race”).
| simplify asyncio's ``staggered_race`` using ``eager_start=False`` | |
| Simplify ``asyncio.staggered_race`` by using ``eager_start=False``. |
| 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. |
There was a problem hiding this comment.
This comment still refers to the removed “ok_to_start” gating (“ok to start”), which is now misleading after switching to eager_start=False. Please update or remove it to reflect the new start semantics.
| # first_task has been appended to running_tasks so first_task is ok to start. | |
| # first_task has been appended to running_tasks before the event loop starts running it. |
| next_task = loop.create_task( | ||
| run_one_coro(this_failed), | ||
| eager_start=False, | ||
| ) |
There was a problem hiding this comment.
By forcing eager_start=False here, staggered_race will no longer exercise the eager-start path even when an eager task factory is installed; existing regression tests named “with eager tasks” may become less meaningful. Consider adding/adjusting a test that asserts staggered_race passes eager_start=False (e.g., via a task factory that records the eager_start kwarg) so this behavior can’t regress silently.
Uh oh!
There was an error while loading. Please reload this page.