Repository navigation
Happy Eyeballs crashes with Eager Task Factory #124309
Copy link
Copy link
Closed
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytopic-asynciotype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Bug report
Bug description:
asyncio's staggered_race crashes when an eager task factory is being used:
import asyncio
async def main():
asyncio.get_running_loop().set_task_factory(asyncio.eager_task_factory)
await asyncio.open_connection("example.com", 80, happy_eyeballs_delay=0.25)
asyncio.run(main())Traceback (most recent call last):
[...]
File "/tmp/repro.py", line 5, in main
await asyncio.open_connection("example.com", 80, happy_eyeballs_delay=0.25)
File "/opt/homebrew/Cellar/python@3.12/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/streams.py", line 48, in open_connection
transport, _ = await loop.create_connection(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/python@3.12/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py", line 1109, in create_connection
sock, _, _ = await staggered.staggered_race(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/python@3.12/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/staggered.py", line 144, in staggered_race
raise d.exception()
File "/opt/homebrew/Cellar/python@3.12/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/staggered.py", line 101, in run_one_coro
assert len(running_tasks) == this_index + 2
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
See also: aio-libs/aiohttp#8599
CPython versions tested on:
3.12
Operating systems tested on:
macOS
Linked PRs
- gh-124309: Fix the AssertionError when using the happy_eyeballs_delay argument in open_connection #124311
- gh-124309: Modernize the
staggered_raceimplementation to support eager task factories #124390 - [3.13] gh-124309: Modernize the
staggered_raceimplementation to support eager task factories (GH-124390) #124573 - [3.12] gh-124309: Modernize the
staggered_raceimplementation to support e… #124574 - gh-124309: Revert eager task factory fix to prevent breaking downstream #124810
- [3.12] gh-124309: Revert eager task factory fix to prevent breaking downstream (GH-124810) #124817
- gh-124309: fix staggered race on eager tasks #124847
- [3.13] gh-124309: fix staggered race on eager tasks (GH-124847) #125339
- [3.12] gh-124309: fix staggered race on eager tasks (GH-124847) #125340
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
3.12only security fixesonly security fixes3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytopic-asynciotype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Projects
- StatusShow more project fieldsDone
Activity
rruuaanng commentedon Sep 22, 2024
I'd love to help you, but what I can't understand is why it creates an extra task that triggers the assertion.
Maybe you can start from here, sorry I'm not familiar with this.
kumaraditya303 commentedon Sep 22, 2024
Probably removing that assert is sufficient.
rruuaanng commentedon Sep 22, 2024
Removing that assertion won't fix the issue; instead, it could cause new errors elsewhere. the real problem isn't with the assertion anyway.
ZeroIntensity commentedon Sep 23, 2024
This issue is unrelated to Happy Eyeballs, but instead that
staggered_racedoesn't supporteager_task_factory. Seperate reproducer:Using -O (to disable assertions) is a workaround for all versions, but maybe it's worth revisiting
staggered_racea little bit to make sure it's working correctly with eager task factory. I'm working on that.ZeroIntensity commentedon Sep 23, 2024
Ah, using
delay=Nonein my reproducer causes a deadlock. Removing the assertion isn't 100% effective.ZeroIntensity commentedon Sep 23, 2024
I've created #124390 as a fix. The existing implementation was so broken for eager task factories (due to the reliance of tasks not starting before you
awaitthem), so I had to pretty much rewrite it.Could you confirm that it solves your problem, @mhils?
rruuaanng commentedon Sep 24, 2024
That's right, I was confused when I tried to fix it, I kept thinking that maybe it was supporting
eager_task_factory.8 remaining items
pythongh-124309: Revert eager task factory fix to prevent breaking do…
ZeroIntensity commentedon Oct 1, 2024
Sorry, we had to revert the fix due to the complexity of it (and also because it was causing some other problems). We'll (hopefully) have the fix for this issue in the next-next 3.12 patch (not tomorrow's, the one after, so 3.12.8).
[3.12] gh-124309: Revert eager task factory fix to prevent breaking d…
gh-124309: fix staggered race on eager tasks (#124847)
pythongh-124309: fix staggered race on eager tasks (pythonGH-124847)
pythongh-124309: fix staggered race on eager tasks (pythonGH-124847)
[3.13] gh-124309: fix staggered race on eager tasks (GH-124847) (#125339
[3.12] gh-124309: fix staggered race on eager tasks (GH-124847) (#125340
graingert commentedon Oct 21, 2024
I think this can be closed, it's backported to 3.12 and 3.13
fix(sources): staggered_race delay 改为 50ms 避免 assertion 崩溃
fix(sources): staggered_race delay 改为 50ms 避免 assertion 崩溃