From 24a062341ebcfcb1c4b1cfe4e861c432e9f32765 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Sun, 2 Apr 2023 14:34:41 -0400 Subject: [PATCH 1/3] Just call `trio.Process.aclose()` directly for now? --- tractor/_spawn.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tractor/_spawn.py b/tractor/_spawn.py index 900aea2..7601c03 100644 --- a/tractor/_spawn.py +++ b/tractor/_spawn.py @@ -206,10 +206,8 @@ async def do_hard_kill( # never release until the process exits, now it acts as # a hard-kill time ultimatum. with trio.move_on_after(terminate_after) as cs: - - # NOTE: This ``__aexit__()`` shields internally. - async with proc: # calls ``trio.Process.aclose()`` - log.debug(f"Terminating {proc}") + log.debug(f"Terminating {proc}") + await proc.aclose() if cs.cancelled_caught: # XXX: should pretty much never get here unless we have From f667d16d66f06027d9536c65678c92b32eb6d8d5 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Sun, 14 May 2023 19:31:50 -0400 Subject: [PATCH 2/3] Copy the now deprecated `trio.Process.aclose()` Move it into our `_spawn.do_hard_kill()` since we do indeed rely on the particular process killing sequence on "soft kill" failure cases. --- tractor/_spawn.py | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/tractor/_spawn.py b/tractor/_spawn.py index 7601c03..e0472a9 100644 --- a/tractor/_spawn.py +++ b/tractor/_spawn.py @@ -23,13 +23,12 @@ import sys import platform from typing import ( Any, + Awaitable, Literal, - Optional, Callable, TypeVar, TYPE_CHECKING, ) -from collections.abc import Awaitable from exceptiongroup import BaseExceptionGroup import trio @@ -60,7 +59,7 @@ if TYPE_CHECKING: log = get_logger('tractor') # placeholder for an mp start context if so using that backend -_ctx: Optional[mp.context.BaseContext] = None +_ctx: mp.context.BaseContext | None = None SpawnMethodKey = Literal[ 'trio', # supported on all platforms 'mp_spawn', @@ -86,7 +85,7 @@ else: def try_set_start_method( key: SpawnMethodKey -) -> Optional[mp.context.BaseContext]: +) -> mp.context.BaseContext | None: ''' Attempt to set the method for process starting, aka the "actor spawning backend". @@ -200,14 +199,37 @@ async def cancel_on_completion( async def do_hard_kill( proc: trio.Process, terminate_after: int = 3, + ) -> None: # NOTE: this timeout used to do nothing since we were shielding # the ``.wait()`` inside ``new_proc()`` which will pretty much # never release until the process exits, now it acts as # a hard-kill time ultimatum. + log.debug(f"Terminating {proc}") with trio.move_on_after(terminate_after) as cs: - log.debug(f"Terminating {proc}") - await proc.aclose() + + # NOTE: code below was copied verbatim from the now deprecated + # (in 0.20.0) ``trio._subrocess.Process.aclose()``, orig doc + # string: + # + # Close any pipes we have to the process (both input and output) + # and wait for it to exit. If cancelled, kills the process and + # waits for it to finish exiting before propagating the + # cancellation. + with trio.CancelScope(shield=True): + if proc.stdin is not None: + await proc.stdin.aclose() + if proc.stdout is not None: + await proc.stdout.aclose() + if proc.stderr is not None: + await proc.stderr.aclose() + try: + await proc.wait() + finally: + if proc.returncode is None: + proc.kill() + with trio.CancelScope(shield=True): + await proc.wait() if cs.cancelled_caught: # XXX: should pretty much never get here unless we have @@ -353,12 +375,11 @@ async def trio_proc( spawn_cmd.append("--asyncio") cancelled_during_spawn: bool = False - proc: Optional[trio.Process] = None + proc: trio.Process | None = None try: try: # TODO: needs ``trio_typing`` patch? - proc = await trio.lowlevel.open_process( # type: ignore - spawn_cmd) + proc = await trio.lowlevel.open_process(spawn_cmd) log.runtime(f"Started {proc}") @@ -442,8 +463,8 @@ async def trio_proc( nursery.cancel_scope.cancel() finally: - # The "hard" reap since no actor zombies are allowed! - # XXX: do this **after** cancellation/tearfown to avoid + # XXX NOTE XXX: The "hard" reap since no actor zombies are + # allowed! Do this **after** cancellation/teardown to avoid # killing the process too early. if proc: log.cancel(f'Hard reap sequence starting for {subactor.uid}') From c53d62d2f752fb28240eddd7d944a3773ef8cfc4 Mon Sep 17 00:00:00 2001 From: Tyler Goodlet Date: Sun, 14 May 2023 20:30:40 -0400 Subject: [PATCH 3/3] Add news file --- nooz/356.trivial.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 nooz/356.trivial.rst diff --git a/nooz/356.trivial.rst b/nooz/356.trivial.rst new file mode 100644 index 0000000..ba0f3f2 --- /dev/null +++ b/nooz/356.trivial.rst @@ -0,0 +1,7 @@ +Drop `trio.Process.aclose()` usage, copy into our spawning code. + +The details are laid out in https://github.com/goodboy/tractor/issues/330. +`trio` changed is process running quite some time ago, this just copies +out the small bit we needed (from the old `.aclose()`) for hard kills +where a soft runtime cancel request fails and our "zombie killer" +implementation kicks in.