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

[Discussion] about freezing asyncio #521

Closed
Mocramis opened this issue Dec 4, 2023 · 7 comments · Fixed by #525
Closed

[Discussion] about freezing asyncio #521

Mocramis opened this issue Dec 4, 2023 · 7 comments · Fixed by #525

Comments

@Mocramis
Copy link

Mocramis commented Dec 4, 2023

PR #470 basically unfroze asyncio during freeze_time.

Although i understand the need for it (not breaking tests that perform real waits on real events), it breaks the main purpose of freeze_time: Asyncio is no-longer frozen even without tick and i can no longer trigger events that would happen in an asyncio future without waiting the full duration( for instance, a common pattern is to do asyncio.sleep(remaining_time)). This makes some tests that were instant have to wait for whole timeout instead with freezegun>=1.3.0.

I understand that both properties may be desirable, but, the first one should only work with tick=True (it is currently inconditional), and may still manipulate monotonic for asyncio (it is desirable to trigger futures events with freezegun).

A switch to get one behaviour or another may be needed; i guess there are even case where monotonic and time may need to be handled in a completely othogonal manner (as manipulating monotonic will always end up breaking a fundamental promise of the api: being monotonic).

So should i make a PR to make this behavior opt-in ? Add a new switch for asyncio ?

(Note, i have a longer take on this on a similar (but note quite the same) issue in time-machine: adamchainz/time-machine#406)

@divad
Copy link

divad commented Dec 5, 2023

This also breaks compatibility with pytest-socket. When using freezegun now, even without any use of asyncio, freezegun triggers an asyncio event loop and this in turn tries to use a socket, which pytest-socket blocks, and the test fails:

  File "/usr/local/lib/python3.11/site-packages/_pytest/python.py", line 194, in pytest_pyfunc_call
    result = testfunction(**testargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/freezegun/api.py", line 816, in wrapper
    with self as time_factory:
  File "/usr/local/lib/python3.11/site-packages/freezegun/api.py", line 634, in __enter__
    return self.start()
           ^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/freezegun/api.py", line 739, in start
    event_loop = asyncio.new_event_loop()
                 ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/asyncio/events.py", line 806, in new_event_loop
    return get_event_loop_policy().new_event_loop()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/asyncio/events.py", line 695, in new_event_loop
    return self._loop_factory()
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/asyncio/unix_events.py", line 64, in __init__
    super().__init__(selector)
  File "/usr/local/lib/python3.11/asyncio/selector_events.py", line 56, in __init__
    self._make_self_pipe()
  File "/usr/local/lib/python3.11/asyncio/selector_events.py", line 107, in _make_self_pipe
    self._ssock, self._csock = socket.socketpair()
                               ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/socket.py", line 609, in socketpair
    a = socket(family, type, proto, a.detach())
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/pytest_socket.py", line 80, in __new__
    raise SocketBlockedError()
pytest_socket.SocketBlockedError: A test tried to use socket.socket.

Since we aren't even using asyncio at all, this is very confusing behaviour. Would it be possible to add a flag or config to freezegun to prevent it from starting asyncio, and thus prevent it from using socket?

@bblommers
Copy link
Collaborator

@divad @Mocramis I'd be happy to review a PR with a flag to opt-out. The current behaviour should still be the default though IMO.

@divad
Copy link

divad commented Dec 5, 2023

My argument would be that users of freezegun who don't use asyncio should not need to opt-out of asyncio - it is unfortunate that freezegun creates an event loop even when the user does not use or wish to use asyncio. However, there seems to be no reliable way to determine if asyncio is in use, and even the asyncio.get_event_loop and similar functions still open a socket, so hit the same issue.

It appears that pytest-socket has their own asyncio workaround, --allow-unix-socket, so I'll use that for now.

@wimglenn
Copy link

wimglenn commented Dec 7, 2023

Even when using --allow-unix-socket I'm still seeing issues, freezegun 1.3+ doesn't play nice with pytest-socket on Windows.

Passing tests with freezegun 1.2.2: https://github.com/wimglenn/advent-of-code-data/actions/runs/7123359485/job/19395845855
Failing tests with freezegun 1.3.1: https://github.com/wimglenn/advent-of-code-data/actions/runs/7123099287/job/19395173162

I encountered this in wimglenn/advent-of-code-data#130 - all other pieces held constant, apart from adding the "freezegun < 1.3" constraint. Note: the project under test does not use asyncio at all.

@divad
Copy link

divad commented Dec 7, 2023

Ah yes, on windows asyncio doesn't use unix sockets / the unix selector, so there is no workaround there as far as I know.

@Mocramis
Copy link
Author

Mocramis commented Dec 8, 2023

I think one of the main reason of the original mr was is that freeze_time does not freeze time.sleep while it freezes asyncio.sleep. The original mr forces the original time.monotonic back on the asyncio loop, but it means that the loop time handling will be both inconsistent with time.monotonic and time.time. (also, if the loop had an intentional different time source, it will still replace it by the original time.monotonic)

However, Wouldn't it make sense for a non-ticking freeze_time to also freeze time.sleep ?

@bblommers
Copy link
Collaborator

The 1.3.0 behaviour is now hidden behind a feature-flag, so freezegun should work as normal unless you explicitly enable this.

with freeze_time('1970-01-02', real_asyncio=True):
    ...

This is part of the 1.4.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants