Repository navigation
fix: bounded timeouts and reliable async teardown, plus three client bugs - #3
Merged
Merged
Conversation
…, and more - Timeout: after killing the process group, collect output for at most kill_grace seconds. A descendant that left the group (setsid) could otherwise hold the pipes open and stall the timeout indefinitely. - _finish: raise the typed QsvError before decoding stdout, so a failed run with non-UTF-8 stdout no longer surfaces as UnicodeDecodeError. - count()/headers(): read output via QsvResult._text(), so they work with stdout_path= and text=False instead of raising IndexError. - find_qsv: return absolute paths, so a relative binary works with cwd=. - Document kill_grace in the client docstring. Each fix has a regression test that fails on the previous code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ary paths Addresses roborev review 5154: - AsyncQsv: proc.wait() also waits for the output pipes to close, so with a SIGTERM-ignoring leader and a descendant that left the group, the post-SIGKILL wait blocked until that descendant exited. Every teardown wait is now bounded by kill_grace. - AsyncQsv: after a timeout or cancellation, close the subprocess transport so pipes held by an escaped descendant no longer keep two fds open per run until it exits, then reap the killed leader. - find_qsv: use Path.absolute() instead of os.path.abspath, which collapsed `..` lexically and pointed at the wrong file after a symlinked directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses roborev review 5155: - A cancellation arriving while the timeout handler was cleaning up (e.g. an outer asyncio deadline during the SIGTERM grace wait) escaped the handler, skipping the SIGKILL and the pipe release; a SIGTERM-ignoring qsv kept running. The timeout cleanup now runs shielded, as the cancellation path already did. - Windows: bound the post-kill wait by kill_grace, since proc.wait() also waits for pipes a surviving child may hold. Untested (no Windows CI). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses roborev review 5157: asyncio.shield() re-raised the cancellation at once and left the cleanup in a background task. When asyncio.run() returned right after an outer deadline, the runner's shutdown cancelled that task mid-SIGTERM-wait, skipping the SIGKILL and leaving a SIGTERM-ignoring qsv running. Both the timeout cleanup and the cancellation path now run the teardown to completion, then re-raise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses roborev review 5158: when asyncio.run() returns while a background run is still in timeout cleanup, the runner cancels the cleanup task directly, mid-SIGTERM-wait, so the SIGKILL and pipe close never ran. Teardown now catches its own cancellation, skips the remaining grace, sends SIGKILL to the group and closes the pipes synchronously, then re-raises. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On Linux with Python 3.12+, asyncio reaps children through a pidfd reader on the event loop. A leader SIGKILLed while asyncio.run() shuts down is therefore never reaped by it, and os.kill(pid, 0) keeps succeeding on the zombie, failing test_asyncio_run_cancelling_background_run_mid_cleanup_still_kills on ubuntu 3.12-3.14 although the process was killed. Check /proc/<pid>/stat for state Z as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 task
jqnatividad
added a commit
that referenced
this pull request
Oct 9, 2026
Bump the version to 0.2.0 in pyproject.toml, __version__ and uv.lock, and move the Unreleased changelog entries under 0.2.0 with PR links: Windows support via Job Objects, bounded timeouts and reliable async teardown, and the client fixes from #3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes from a codebase review, plus follow-ups from automated (roborev) reviews of those fixes. Every fix has a regression test that fails on the previous code.
Process teardown (
_process.py)setsid), it held the output pipes open, andtimeout=1waited for that process to exit. After a kill, output is now collected for at mostkill_grace. This also holds when qsv ignores SIGTERM. On Python 3.14, asyncio'sproc.wait()waits for the pipes to close, so every teardown wait is now bounded.AsyncQsvcloses its pipe transport. Before, each run whose pipes were held by such a process leaked 2 fds until that process exited.AsyncQsvrun, including mid-cleanup or throughasyncio.run()shutdown, now SIGKILLs the process group. The teardown finishes before the cancellation propagates, and if the teardown itself is cancelled it sends SIGKILL and closes the pipes synchronously.Client (
client.py)QsvErrorinstead ofUnicodeDecodeError.count()/headers()work withstdout_path=andtext=Falseinstead of raisingIndexError.find_qsvreturns absolute paths, so a relativebinaryworks together withcwd=. It usesPath.absolute(), which leaves..alone and so stays correct after a symlinked directory.kill_grace.CI
Test plan
uv run pytest: 55 passed on Python 3.14, also with-W error::ResourceWarningtests/test_process.pyon Python 3.10ruff format --check,ruff check,mypy(strict) clean🤖 Generated with Claude Code