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

[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. 🍰 😃