Skip to content

Commit a070356

Browse files
committed
Guard double-terminate on returncode; make _terminate_and_reap idempotent
Addresses review feedback: calling _terminate_process_tree on an already-reaped POSIX process hits the fallback path in terminate_posix_process_tree (os.getpgid on a stale PID raises ProcessLookupError), which emits two WARNING logs and an ERROR with traceback. These are visible because pyproject.toml sets log_cli=true. Previously, each test called _terminate_process_tree in the body and the AsyncExitStack called _terminate_and_reap (which re-called _terminate_process_tree) at exit — double-terminate, nine spurious log records per run. Fix: _terminate_and_reap now checks proc.returncode before terminating. On POSIX, the asyncio child watcher sets returncode during _terminate_process_tree's polling-loop yield, so by the time cleanup runs it's populated and the guard skips the noisy re-terminate. On Windows/FallbackProcess (no returncode attribute), getattr returns None and terminate runs — but the Windows path is silent on double-call. Restructured tests to call _terminate_and_reap (which wraps _terminate_process_tree) as the action under test, and let the ExitStack safety-net call it again. Since wait() and stream.aclose() are idempotent, this is safe — and it covers both branches of the guard (first call: returncode is None → terminate; second call: returncode set → skip). 100% coverage, still zero pragmas. Github-Issue: #1775
1 parent 54295dd commit a070356

File tree

1 file changed

+15
-8
lines changed

1 file changed

+15
-8
lines changed

tests/client/test_stdio.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,17 @@ async def _terminate_and_reap(proc: anyio.abc.Process | FallbackProcess) -> None
321321
reaping + stream closure. These tests call ``_terminate_process_tree``
322322
directly, so they must do the same cleanup explicitly.
323323
324-
Safe to call on an already-terminated process (termination no-ops, wait
325-
returns immediately). Bounded by ``move_on_after`` to prevent hangs.
324+
Idempotent: the ``returncode`` guard skips termination if the process has
325+
already been reaped (calling ``_terminate_process_tree`` on a reaped POSIX
326+
process hits the fallback path in ``terminate_posix_process_tree`` and emits
327+
spurious WARNING/ERROR logs, visible because ``log_cli = true``); ``wait()``
328+
and stream ``aclose()`` are no-ops on subsequent calls. The tests call this
329+
explicitly as the action under test, and ``AsyncExitStack`` calls it again
330+
on exit as a safety net for early failures. Bounded by ``move_on_after``.
326331
"""
327332
with anyio.move_on_after(5.0):
328-
await _terminate_process_tree(proc)
333+
if getattr(proc, "returncode", None) is None:
334+
await _terminate_process_tree(proc)
329335
await proc.wait()
330336
assert proc.stdin is not None
331337
assert proc.stdout is not None
@@ -356,8 +362,9 @@ async def test_basic_child_process_cleanup(self):
356362
stream = await _accept_alive(sock)
357363
stack.push_async_callback(stream.aclose)
358364

359-
# Terminate the process tree (the behavior under test).
360-
await _terminate_process_tree(proc)
365+
# Terminate, reap and close transports (wraps _terminate_process_tree,
366+
# the behavior under test).
367+
await _terminate_and_reap(proc)
361368

362369
# Deterministic: kernel closed child's socket when it died.
363370
await _assert_stream_closed(stream)
@@ -391,8 +398,8 @@ async def test_nested_process_tree(self):
391398
stack.push_async_callback(stream.aclose)
392399
streams.append(stream)
393400

394-
# Terminate the entire tree.
395-
await _terminate_process_tree(proc)
401+
# Terminate the entire tree (wraps _terminate_process_tree).
402+
await _terminate_and_reap(proc)
396403

397404
# Every level of the tree must be dead: three kernel-level EOFs.
398405
for stream in streams:
@@ -426,7 +433,7 @@ async def test_early_parent_exit(self):
426433

427434
# Parent will sys.exit(0) on SIGTERM, but the process-group kill
428435
# (POSIX killpg / Windows Job Object) must still terminate the child.
429-
await _terminate_process_tree(proc)
436+
await _terminate_and_reap(proc)
430437

431438
# Child must be dead despite parent's early exit.
432439
await _assert_stream_closed(stream)

0 commit comments

Comments
 (0)