Happy Eyeballs crashes with Eager Task Factory

[mhils]

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_race implementation to support eager task factories #124390
• [3.13] gh-124309: Modernize the staggered_race implementation to support eager task factories (GH-124390) #124573
• [3.12] gh-124309: Modernize the staggered_race implementation 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

[rruuaanng]

I'd love to help you, but what I can't understand is why it creates an extra task that triggers the assertion.

next_task = loop.create_task(run_one_coro(this_failed))
running_tasks.append(next_task)
print('\n', 'HELLO', len(running_tasks), this_index)
assert len(running_tasks) == this_index + 2
# Prepare place to put this coroutine's exceptions if not won
exceptions.append(None)
assert len(exceptions) == this_index + 1

 HELLO 1 1
 HELLO 3 2

Maybe you can start from here, sorry I'm not familiar with this.

[kumaraditya303]

Probably removing that assert is sufficient.

[rruuaanng]

Probably removing that assert is sufficient.

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]

This issue is unrelated to Happy Eyeballs, but instead that staggered_race doesn't support eager_task_factory. Seperate reproducer:

import asyncio
import random

async def main():
    asyncio.get_running_loop().set_task_factory(asyncio.eager_task_factory)
    await asyncio.staggered.staggered_race([lambda: asyncio.sleep(0) for _ in range(5)], 0.25)

asyncio.run(main())

Using -O (to disable assertions) is a workaround for all versions, but maybe it's worth revisiting staggered_race a little bit to make sure it's working correctly with eager task factory. I'm working on that.

[ZeroIntensity]

Ah, using delay=None in my reproducer causes a deadlock. Removing the assertion isn't 100% effective.

[ZeroIntensity]

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 await them), so I had to pretty much rewrite it.

Could you confirm that it solves your problem, @mhils?

[rruuaanng]

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 await them), so I had to pretty much rewrite it.

Could you confirm that it solves your problem, @mhils?

That's right, I was confused when I tried to fix it, I kept thinking that maybe it was supporting eager_task_factory .

[mhils]

Thank you so much @ZeroIntensity! Works perfectly. 🍰 😃

[ZeroIntensity]

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).

[graingert]

I think this can be closed, it's backported to 3.12 and 3.13