-
Notifications
You must be signed in to change notification settings - Fork 204
Add cooperative signal timeouts #1548
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
Open
tamird
wants to merge
12
commits into
pytest-dev:main
Choose a base branch
from
tamird:tamird/pytest-timeout-integration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
58aed64
Add cooperative signal timeouts
tamird b96aff4
Simplify cooperative timeout integration
tamird 4a319fd
Remove redundant timeout bookkeeping
tamird 2b9fc33
Simplify timeout delivery and test isolation
tamird fab62f5
Preserve coroutine ownership across interrupts
tamird 7c8ea75
Discover timeout support when runners start
tamird 8d7f18a
Defer coroutine creation to the running task
tamird 0de5523
Cover timeouts on free-threaded Python
tamird 86efcea
Run timeout adapter tests without a subprocess
tamird fd2a673
Limit cooperative delivery to the main thread
tamird 3ef5eb3
Run timeout delivery through its owner
tamird f729f0f
Cover native timeouts across pytest versions
tamird File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Automatically deliver pytest-timeout signal failures cooperatively in asynchronous tests and fixtures on Python 3.11 and newer when pytest-timeout provides its expiry hook. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ Reference | |
| fixtures/index | ||
| functions | ||
| hooks | ||
| timeouts | ||
| markers/index | ||
| decorators/index | ||
| changelog | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| ======== | ||
| Timeouts | ||
| ======== | ||
|
|
||
| On Python 3.11 and newer, pytest-asyncio automatically delivers pytest-timeout | ||
| signal failures by cancelling the active asynchronous test or fixture when | ||
| pytest-timeout provides the ``pytest_timeout_expired`` hook. Cooperative cleanup | ||
| can then run before pytest reports the original timeout. No configuration is | ||
| needed. Python 3.10 and older pytest-timeout versions retain their existing | ||
| signal behavior. Cooperative test execution requires a native ``async def`` | ||
| function, bound method, or plain ``functools.partial`` of one. Synchronous | ||
| coroutine creators, including functions marked with | ||
| ``inspect.markcoroutinefunction()`` and subclasses of ``functools.partial``, | ||
| retain their existing call timing, context, and synchronous signal delivery. | ||
| pytest-timeout still controls the configured duration, covered test phases, | ||
| debugger detection, and timeout diagnostics. | ||
|
|
||
| Cancellation waits for the event loop and coroutine to cooperate. A raised | ||
| signal exception can interrupt CPU-bound Python code, but cooperative | ||
| cancellation cannot interrupt code that never yields. It also cannot stop | ||
| an indefinitely blocking callback or a task that refuses cancellation. | ||
| Use pytest-timeout's ``thread`` method or an independent process watchdog when | ||
| the process must be terminated; these stop the entire process without normal | ||
| test teardown. A timeout during final event-loop shutdown stops that shutdown | ||
| and closes the loop; remaining resource cleanup may be incomplete. | ||
|
|
||
| The integration does not take over runners managed by other async plugins or | ||
| synchronous tests that call ``asyncio.run()`` themselves. It wraps asynchronous | ||
| tests and fixtures in a timeout context, so ``asyncio.current_task().get_coro()`` | ||
| returns the wrapper coroutine rather than the original test coroutine. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| """Cooperative delivery of pytest-timeout's signal failures.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import contextvars | ||
| import sys | ||
| import threading | ||
| from collections.abc import Callable, Coroutine | ||
| from dataclasses import dataclass | ||
| from typing import Any, TypeVar | ||
|
|
||
| import pytest | ||
|
|
||
| __tracebackhide__ = True | ||
| _T = TypeVar("_T") | ||
|
|
||
|
|
||
| @dataclass | ||
| class _Delivery: | ||
| config: pytest.Config | ||
| loop: asyncio.AbstractEventLoop | ||
| closing: bool = False | ||
| exception: BaseException | None = None | ||
| timeout: asyncio.Timeout | None = None | ||
|
|
||
| def run(self, operation: Callable[[], _T]) -> _T: | ||
| previous = self.config.stash.get(_CURRENT_DELIVERY, None) | ||
| try: | ||
| try: | ||
| self.config.stash[_CURRENT_DELIVERY] = self | ||
| result = operation() | ||
| finally: | ||
| # Once the runner returns, a new signal can fail synchronously. | ||
| # Stop claiming it before deciding which outcome to propagate. | ||
| self.config.stash[_CURRENT_DELIVERY] = previous | ||
| except (KeyboardInterrupt, SystemExit, pytest.exit.Exception): | ||
| raise | ||
| except asyncio.CancelledError as exc: | ||
| if self.exception is None: | ||
| raise | ||
| # asyncio.Timeout converts only its own cancellation to TimeoutError. | ||
| # Preserve cancellation requested by another caller. | ||
| raise exc from self.exception | ||
| except BaseException as exc: | ||
| if self.exception is None or exc is self.exception: | ||
| raise | ||
| raise self.exception from exc | ||
| if self.exception is not None: | ||
| raise self.exception | ||
| return result | ||
|
|
||
| def interrupt(self) -> None: | ||
| if self.config.stash.get(_CURRENT_DELIVERY, None) is not self: | ||
| return | ||
| if self.closing: | ||
| # A completed shutdown phase can consume stop(). Keep stopping | ||
| # until Runner.close() returns; never stop a reusable invocation. | ||
| self.loop.stop() | ||
| self.loop.call_soon(self.interrupt) | ||
| elif self.timeout is not None: | ||
| self.timeout.reschedule(self.loop.time()) | ||
|
|
||
|
|
||
| # SIGALRM only reaches the main thread; worker runners keep their native behavior. | ||
| _CURRENT_DELIVERY = pytest.StashKey[_Delivery | None]() | ||
|
|
||
|
|
||
| def _supports_cooperative_timeouts(config: pytest.Config) -> bool: | ||
| # Test modules can load pytest-timeout after pytest_configure has run. | ||
| return ( | ||
| sys.version_info >= (3, 11) | ||
| and threading.current_thread() is threading.main_thread() | ||
| and config.hook.pytest_timeout_expired.has_spec() | ||
| ) | ||
|
|
||
|
|
||
| @pytest.hookimpl(tryfirst=True, optionalhook=True) | ||
| def pytest_timeout_expired(item: pytest.Item, exception: BaseException) -> bool | None: | ||
| if threading.current_thread() is not threading.main_thread(): | ||
| return None | ||
| invocation = item.config.stash.get(_CURRENT_DELIVERY, None) | ||
| if invocation is None: | ||
| return None | ||
| if invocation.exception is None: | ||
| invocation.exception = exception | ||
| # Raising here can interrupt asyncio before it schedules a task's next | ||
| # step. Return to the interrupted code and cancel at a safe loop turn. | ||
| # Late callbacks check ownership instead of relying on Handle.cancel(): | ||
| # SIGINT can interrupt scheduling before the handle is returned. | ||
| if not invocation.loop.is_closed(): | ||
| invocation.loop.call_soon_threadsafe(invocation.interrupt) | ||
| return True | ||
|
|
||
|
|
||
| def run( | ||
| runner: asyncio.Runner, | ||
| coro_factory: Callable[[], Coroutine[Any, Any, _T]], | ||
| *, | ||
| context: contextvars.Context, | ||
| config: pytest.Config, | ||
| ) -> _T: | ||
| """Run a native coroutine factory with cooperative timeout delivery.""" | ||
| if not _supports_cooperative_timeouts(config): | ||
| return runner.run(coro_factory(), context=context) | ||
|
|
||
| invocation = _Delivery(config, runner.get_loop()) | ||
|
|
||
| async def invoke() -> _T: | ||
|
tjkuson marked this conversation as resolved.
|
||
| if invocation.exception is not None: | ||
| raise invocation.exception | ||
| try: | ||
| async with asyncio.timeout(None) as timeout: | ||
| invocation.timeout = timeout | ||
| # Create the user coroutine only once its task owns execution. | ||
| # Runner and task factories retain ownership of invoke(). | ||
| return await coro_factory() | ||
| finally: | ||
| # The signal may have queued delivery just as the coroutine exits. | ||
| # Do not reschedule a timeout whose context has already exited. | ||
| invocation.timeout = None | ||
|
|
||
| return invocation.run(lambda: runner.run(invoke(), context=context)) | ||
|
|
||
|
|
||
| def close(runner: asyncio.Runner, *, config: pytest.Config) -> None: | ||
| if not _supports_cooperative_timeouts(config): | ||
| runner.close() | ||
| return | ||
| _Delivery(config, runner.get_loop(), closing=True).run(runner.close) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runner.close()runs several blocking phases, each viarun_until_complete. If the timeout lands as one phase completes, that phase consumes the stop andclose()can proceed into a blockingshutdown_default_executor()after the only alarm has fired and hang indefinitely. We might need some way to persist the stop...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b96aff4. The stop callback now requeues itself until
Runner.close()exits, so completing one shutdown phase cannot consume the only stop and leave the next phase blocked. The remaining callback is canceled on exit. The regression triggers expiry at the async-generator/executor shutdown boundary with an executor job still blocked; it passes with this fix and fails with the previous one-shot stop.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up on the callback detail: on the current head, the stop still requeues across shutdown phases until
Runner.close()exits. There is no saved handle to cancel now: callbacks check that their delivery is still active and otherwise return. This avoids relying on receiving a handle before SIGINT interrupts scheduling. The async-generator/executor boundary regression still passes locally.Posted with Codex assistance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this addresses the race I was concerned about, though I will leave this thread open given it's probably the most nuanced thing in this PR and will think about it some more.