Add cooperative signal timeouts - #1548
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1548 +/- ##
==========================================
+ Coverage 94.50% 95.19% +0.68%
==========================================
Files 2 3 +1
Lines 510 603 +93
Branches 62 78 +16
==========================================
+ Hits 482 574 +92
+ Misses 22 19 -3
- Partials 6 10 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d36b012 to
c149e1f
Compare
A signal timeout can interrupt asyncio while it schedules a task's next step, leaving the task pending without a continuation [1]. Catching the resulting pytest failure and cancelling that task cannot reliably recover it. Add opt-in cooperative delivery through pytest-timeout's proposed expiry hook [2]. Queue cancellation without raising into the scheduler, then report the supplied failure at the owned runner boundary. Preserve external cancellation, process-control exceptions, and the interrupted await's traceback. Stop final runner shutdown separately when it times out; ordinary shared-runner invocations remain reusable. Keep timer selection, deadlines, debugger handling, and diagnostics in pytest-timeout. Existing signal behavior remains the default. Cooperative cancellation cannot stop blocking or cancellation-resistant code, so the thread method remains the hard process-stop option. Final runner shutdown may leave resource cleanup incomplete. Pin the producer prerequisite in a dedicated integration environment until its hook is released. Collect coverage from the isolated pytest processes and map installed-wheel paths back to the source tree. Require Coverage.py 7.10.3 for its subprocess-directory fixes [3]. [1]: pytest-dev/pytest-timeout#113 [2]: pytest-dev/pytest-timeout#204 [3]: https://coverage.readthedocs.io/en/7.10.3/changes.html
c149e1f to
58aed64
Compare
tjkuson
left a comment
tjkuson
left a comment
There was a problem hiding this comment.
Hi, thanks for the PR! Improving how timeouts for users of pytest-asyncio and pytest-timeout would be great, and this new hook is an interesting proposal.
I've left some comments from a pytest-asyncio angle. I'm not directly involved with the pytest-timeout plugin and can't speak on their behalf regarding whether this new hook works for them.
| the timeout is chained to it, because the runner cannot reliably distinguish | ||
| timeout cancellation from another cancellation request. | ||
|
|
||
| Cooperative timeouts are disabled by default. Enabling them without a |
There was a problem hiding this comment.
I am curious, what was the motivation for this to be disabled by default and enabled via a configuration? What would be the reason for a user of both pytest-asyncio and pytest-timeout not enable this feature?
Perhaps I am mistaken, but I see this PR more as a compatibility layer for a (possible) pytest-timeout hook rather than something a user would want to configure. As a maintainer, I'd rather say "this plugin integrates with pytest-timeout" instead of "you need to enable these configurations to get these plugins to work together."
There was a problem hiding this comment.
The opt-in was intended to preserve interruption semantics: raising from SIGALRM can interrupt CPU-bound Python code, whereas cooperative cancellation needs the event loop to run. I agree that requiring another configuration switch is undesirable.
I've now made the integration automatic on Python 3.11+ when the expiry hook is available, and the documentation makes that tradeoff explicit. Users needing hard termination can use the thread method, which terminates the whole process rather than continuing the test run.
| if invocation.exception is not None: | ||
| coro.close() | ||
| raise invocation.exception | ||
| # A custom Python 3.10 task factory can bypass the runner backport's |
There was a problem hiding this comment.
This seems like quite a lot of work to support 3.10 which is nearly EOL and does not need to be supported by pytest-asyncio for much longer. The PR that removes 3.10 support should probably be its own thing given we have 3.10-specific code already in the codebase, but for the purpose of this PR I am in favour of just treating 3.10 as unsupported. It should simplify things.
Likewise, we could use asyncio.timeout and asyncio.Timeout.
There was a problem hiding this comment.
Agreed. The integration now uses asyncio.timeout(None) on Python 3.11+, with expiry rescheduled from the queued event-loop callback. That lets the standard library own the cancellation accounting and removes the custom Python 3.10 task fallback. Python 3.10 retains the existing signal behavior; the project's supported Python versions are unchanged.
|
|
||
| invocation = _Invocation(runner.get_loop()) | ||
|
|
||
| async def invoke() -> _T: |
There was a problem hiding this comment.
This wrapper changes the test coroutine which messes with the traceback and changes task.get_coro. The first could probably be solved by setting __tracebackhide__, but I am not sure if anything can be done about the second issue (though I am also not convinced that it's blocking given we already wrap fixtures).
There was a problem hiding this comment.
Added __tracebackhide__ to the internal delivery frames and checked that the interrupted application frame remains visible. The get_coro() difference remains: the timeout context has to be entered inside the task executing the test, so this design still needs the wrapper. That behavior is now documented. I'd prefer to keep the wrapper rather than take over task creation or introduce another task just to preserve coroutine introspection.
| if state.invocation is self: | ||
| # Runner.close() owns the loop and closes it in a finally block. | ||
| # Stop only that final shutdown, never a reusable runner invocation. | ||
| self.loop.stop() |
There was a problem hiding this comment.
Runner.close() runs several blocking phases, each via run_until_complete. If the timeout lands as one phase completes, that phase consumes the stop and close() can proceed into a blocking shutdown_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.
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.
Use asyncio.Timeout to own cancellation accounting on Python 3.11+ and activate the integration automatically when pytest-timeout provides its expiry hook [1]. Leave Python 3.10 on its existing signal path and remove the opt-in configuration and custom-task cancellation fallback. Document that cooperative cancellation cannot preempt Python code that never yields, while the thread method terminates the whole process. A stop at the boundary between Runner.close() phases can be consumed without interrupting the next phase [2]. Keep the stop callback queued while the final shutdown is active, and cancel its remaining handle on exit. This prevents later executor shutdown from outliving an already received expiry without introducing another timer. Hide internal delivery frames while preserving the interrupted await's traceback. Document the remaining coroutine wrapper rather than changing task creation to conceal it. [1]: https://docs.python.org/3/library/asyncio-task.html#asyncio.timeout [2]: pytest-dev#1548 (comment)
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
full disclosure: i'm doing by best but the intricacies of asyncio are beyond the depth of my knowledge so i'm leaning on codex pretty hard.
if at any point you feel like you're better off picking this up do let me know, I won't be offended. I still haven't heard anything on the pytest-timeout of this proposal.
Thanks for reviewing!
| the timeout is chained to it, because the runner cannot reliably distinguish | ||
| timeout cancellation from another cancellation request. | ||
|
|
||
| Cooperative timeouts are disabled by default. Enabling them without a |
There was a problem hiding this comment.
The opt-in was intended to preserve interruption semantics: raising from SIGALRM can interrupt CPU-bound Python code, whereas cooperative cancellation needs the event loop to run. I agree that requiring another configuration switch is undesirable.
I've now made the integration automatic on Python 3.11+ when the expiry hook is available, and the documentation makes that tradeoff explicit. Users needing hard termination can use the thread method, which terminates the whole process rather than continuing the test run.
| if invocation.exception is not None: | ||
| coro.close() | ||
| raise invocation.exception | ||
| # A custom Python 3.10 task factory can bypass the runner backport's |
There was a problem hiding this comment.
Agreed. The integration now uses asyncio.timeout(None) on Python 3.11+, with expiry rescheduled from the queued event-loop callback. That lets the standard library own the cancellation accounting and removes the custom Python 3.10 task fallback. Python 3.10 retains the existing signal behavior; the project's supported Python versions are unchanged.
|
|
||
| invocation = _Invocation(runner.get_loop()) | ||
|
|
||
| async def invoke() -> _T: |
There was a problem hiding this comment.
Added __tracebackhide__ to the internal delivery frames and checked that the interrupted application frame remains visible. The get_coro() difference remains: the timeout context has to be entered inside the task executing the test, so this design still needs the wrapper. That behavior is now documented. I'd prefer to keep the wrapper rather than take over task creation or introduce another task just to preserve coroutine introspection.
| if state.invocation is self: | ||
| # Runner.close() owns the loop and closes it in a finally block. | ||
| # Stop only that final shutdown, never a reusable runner invocation. | ||
| self.loop.stop() |
There was a problem hiding this comment.
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.
Cancel delivery handles at their owning boundary instead of clearing them as callbacks run. Keep the active-invocation checks: a signal can interrupt scheduling after a callback is queued but before its handle is saved. Use asyncio.Runner in the helper's postponed annotations and import it directly in the Python 3.11+ startup regression. Remove the fieldless dataclass redeclaration and the subprocess smoke test already covered by test_asyncio_marker in the ordinary plugin-free environments.
Collapse delivery state into one record with an explicit shutdown mode. Use the active invocation check to ignore stale callbacks and remove saved-handle cancellation. The check also covers SIGINT interrupting scheduling before a handle is returned. Queued callbacks retain their delivery until processed or the loop closes. Batch startup and outcome checks around the real Runner boundary, and group the public shared-loop and fixture cases in separate child sessions. This preserves the failure cases and subprocess watchdogs without launching an interpreter for each parameter.
The startup cleanup in 58aed64 confuses first execution with Task ownership. An interrupt after Task creation can therefore close a wrapper that the Task still owns. Interruption inside timeout setup can instead leave the original coroutine unawaited. Use the actual pending Task to decide when cleanup is safe, and close only native coroutines that have never started. Closing a suspended coroutine can execute user cleanup synchronously and mask KeyboardInterrupt. Keep custom Coroutine objects on the plain Runner path. Their interface exposes no execution state, so wrapping them cannot preserve safe cleanup during interrupted startup.
Test modules can load pytest-timeout through pytest_plugins during collection, after pytest-asyncio has already configured itself. The configure-time check in 58aed64 then leaves managed runners on the synchronous fallback even though the expiry hook is available. Check hook support when entering a runner and initialize delivery state only when needed. This also removes registration-order-dependent configuration plumbing.
A task factory may run the supplied coroutine inside its own wrapper. The scan added in fab62f5 then misses the owning Task and can close a live coroutine after SIGINT, breaking subsequent fixture teardown. Create native user coroutines inside the running timeout task instead. Leave the Runner-input coroutine with Runner and its task factory, removing the extra lifetime and speculative cleanup. Check the original callable before binding arguments. Marked synchronous creators and partial subclasses must retain caller-side execution and legacy signal delivery. Their custom call behavior can otherwise move into the task or disappear when functools.partial flattens its input. Exercise pre-entry expiry and indirect ownership, replacing tests for states eliminated by lazy creation.
The free-threaded CI job has no tox-gh-actions mapping and falls back to the generic py environment. Pytest-timeout is absent there, so the job passes while skipping every timeout integration test. Add an explicit py314t environment and select it alongside the pinned pytest-timeout environment for Python 3.14t. This preserves baseline coverage and exercises the integration with the free-threaded build.
Only the real signal and shutdown regressions need process isolation. The direct runner checks use an unbounded wait, which makes missing cancellation hang instead of producing an assertion failure. Bound that wait and explicitly verify cancellation, then collect those cases as ordinary tests. This removes a generated test module and a pytest subprocess while exposing the code to normal lint and typing checks.
Pytest-timeout invokes the expiry hook from SIGALRM and uses its terminating thread method when timers are started off the main thread. Worker runners therefore have no cooperative signal-delivery path. Keep only the active main-thread invocation in the config stash. This removes thread-local state and leaves worker runners unwrapped. Both runner entry and the expiry hook check the thread, so a foreign hook call cannot claim the main thread's timeout.
3 participants
A signal timeout can interrupt asyncio while it schedules a task's next step, leaving a pending task with no continuation 1. Catching the resulting failure and cancelling that task cannot reliably recover it.
Add opt-in cooperative signal timeouts through the proposed pytest-timeout expiry hook 2. The signal handler records the supplied failure and queues cancellation without raising into the scheduler. The owned runner then reports the timeout with the interrupted await's traceback, while preserving process-control exceptions and external cancellation. Final runner shutdown has a separate interruption path that closes the loop and may leave resource cleanup incomplete.
Existing signal behavior remains the default. Cooperative cancellation cannot stop an indefinitely blocking callback or a task that refuses to finish; pytest-timeout's thread method remains the hard process-stop option. This consumer does not take over other async plugins, synchronous tests that call asyncio.run(), retry budgets, or background-task ownership.
This keeps runner integration separate from timer policy, as requested in the earlier timeout discussion 3. The dedicated integration environment pins the producer prerequisite; that pin should become a released test dependency once the hook is available. Related to #215.
Prepared with Codex assistance.