Skip to content
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

Infinite loop when used with async_solipsism #10149

Closed
1 task done
bmerry opened this issue Dec 9, 2024 · 2 comments · Fixed by #10151
Closed
1 task done

Infinite loop when used with async_solipsism #10149

bmerry opened this issue Dec 9, 2024 · 2 comments · Fixed by #10151
Labels

Comments

@bmerry
Copy link
Contributor

bmerry commented Dec 9, 2024

Describe the bug

async_solipsism is an event loop that uses fake time - good for unit tests. The keepalive logic in aiohttp assumes that loop.call_at will always call the handler strictly later than the requested time, but that doesn't hold for the idealised time source in async_solipsism. The net result is that tests using async_solipsism can get stuck in an infinite loop where aiohttp keeps re-submitting the keepalive handler.

To Reproduce

  1. pip install aiohttp==3.11.10 async-solipsism==0.7 pytest==8.2.2 pytest-asyncio==0.24.0
  2. Put the following in pytest.ini:
[pytest]
asyncio_mode = auto
asyncio_default_fixture_loop_scope = function
  1. Put the following in test_stuff.py:
import asyncio

import async_solipsism
import pytest
from aiohttp import web, test_utils


@pytest.fixture
def event_loop_policy():
    return async_solipsism.EventLoopPolicy()


@pytest.fixture(autouse=True)
def mock_start_connection(monkeypatch):
    monkeypatch.setattr("aiohappyeyeballs.start_connection",
                        async_solipsism.aiohappyeyeballs_start_connection)


def socket_factory(host, port, family):
    return async_solipsism.ListenSocket((host, port))


async def test_integration():
    app = web.Application()
    async with test_utils.TestServer(app, socket_factory=socket_factory) as server:
        async with test_utils.TestClient(server) as client:
            resp = await client.post("/hey", json={})
            assert resp.status == 404
            await asyncio.sleep(10000)
            resp = await client.post("/hey", json={})
            assert resp.status == 404
  1. Run pytest. The test hangs.

Expected behavior

The test should succeed.

Logs/tracebacks

======================================= test session starts ========================================
platform linux -- Python 3.12.3, pytest-8.2.2, pluggy-1.5.0
rootdir: /home/bmerry/work/bugs/aiohttp-solipsism
configfile: pytest.ini
plugins: mock-3.12.0, timeout-2.3.1, reportlog-0.4.0, asyncio-0.24.0, repeat-0.9.3, xdist-3.5.0, cov-4.1.0, anyio-4.3.0, check-2.2.2, custom-exit-code-0.3.0
asyncio: mode=Mode.AUTO, default_loop_scope=function
collected 1 item                                                                                   

test_stuff.py

Python Version

$ python --version
Python 3.12.3

aiohttp Version

3.11.10

multidict Version

6.0.5

propcache Version

0.2.0

yarl Version

1.18.0

OS

Ubuntu 24.04

Related component

Server

Additional context

The issue is in this line. If the handler fires exactly on time, this check treats it as too early, and it is queued to run again. It will keep getting re-submitted until the clock ticks past close_time. With a real system that's likely to happen immediately (since the number of instructions that can run per clock tick is pretty small), but with the artificial time in async_solipsism (and probably any similar source of mock time), time doesn't flow until there is something to sleep for, and this becomes an infinite loop.

I'm happy to supply a patch. The fix itself is easy (change <= to <), but let me know what you recommend for regression testing. Would it be reasonable to make async_solipsism a dev dependency of aiohttp so that it can be used in a regression test?

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@bmerry bmerry added the bug label Dec 9, 2024
@bdraco
Copy link
Member

bdraco commented Dec 9, 2024

Changing it from <= to < is unlikely to have a material effect on production since its in microseconds and its likely below the resolution of the clock anyways. So thats probably fine and makes sense.

@bmerry
Copy link
Contributor Author

bmerry commented Dec 9, 2024

Changing it from <= to < is unlikely to have a material effect on production since its in microseconds and its likely below the resolution of the clock anyways. So thats probably fine and makes sense.

Thanks, that was my feeling too but it's good to have confirmation. I'll send a PR tomorrow.

bmerry added a commit to bmerry/aiohttp that referenced this issue Dec 10, 2024
bmerry added a commit to bmerry/aiohttp that referenced this issue Dec 10, 2024
bdraco pushed a commit that referenced this issue Dec 17, 2024
…ime is not moving forward (#10173)

Co-authored-by: Bruce Merry <[email protected]>
Fixes #123'). -->
Fixes #10149.
bdraco pushed a commit that referenced this issue Dec 17, 2024
…ime is not moving forward (#10174)

Co-authored-by: Bruce Merry <[email protected]>
Fixes #123'). -->
Fixes #10149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants