diff --git a/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.md b/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.md new file mode 100644 index 00000000..b3ee4bba --- /dev/null +++ b/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.md @@ -0,0 +1,47 @@ +--- +model: gpt-5.6-sol +service: opencode +session: trio-pre-handshake-child-death +timestamp: 2026-08-28T04:51:19Z +git_ref: fb6d81d3 +scope: code +substantive: true +raw_file: 20260828T045119Z_fb6d81d3_prompt_io.raw.md +--- + +## Prompt + +The human approved the next WireGuard Layer C implementation step after +committing child-side network namespace bootstrap. + +## Response summary + +Restore and harden the boot-race design originally implemented by +`3b0724eba85b4014170ed95773e1e41a60d5c513` before transferring +namespace FDs. Race the initial peer handshake against child-process +death so a pre-connection bootstrap failure surfaces instead of +parking `ActorNursery.start_actor()` forever. + +## Files changed + +- `tractor/spawn/_spawn.py` - provide the generic handshake/process + death race. +- `tractor/spawn/_trio.py` - use the race during Trio child startup. +- `tests/test_spawning.py` - cover handshake-first and death-first + schedules, simultaneous outcomes, waiter errors, and backend cleanup. + +## Human edits + +The human approved continued Layer C implementation. The agent narrowed +the next slice to failure detection after analysis showed FD transfer +would otherwise introduce a parent hang. The human then identified the +earlier `wait_for_peer_or_proc_death()` implementation and required +explicit credit; the source docstring and commit message now cite its +original commit rather than a later cherry-pick. The human directly +reflowed `_trio.py` to the project line limit and normalized nearby +quotes. The human also required naming the race helper in its lifecycle +comment, explicit `trio.Cancelled` handling in waiter tests, expected +cancellation diagnostics, and a `trio.Process`-constrained test double. +After mainline portability verification, the human chose to commit the +fix on the current WireGuard branch and require its cherry-pick onto +`main` as a blocker in the eventual WireGuard pull request. diff --git a/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.raw.md b/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.raw.md new file mode 100644 index 00000000..865560f6 --- /dev/null +++ b/ai/prompt-io/opencode/20260828T045119Z_fb6d81d3_prompt_io.raw.md @@ -0,0 +1,34 @@ +--- +model: gpt-5.6-sol +service: opencode +timestamp: 2026-08-28T04:51:19Z +git_ref: fb6d81d3 +diff_cmd: git diff HEAD~1..HEAD +--- + +# Raw output - detect child death before parent handshake + +The human asked to continue WireGuard Layer C after child entrypoints +learned to consume inherited network namespace capabilities. + +## Generated code + +> `git diff HEAD~1..HEAD -- tractor/spawn/_spawn.py tractor/spawn/_trio.py tests/test_spawning.py` + +Race a spawned child's initial parent handshake against its process +wait operation. Return the established channel when the handshake wins; +raise `ActorFailure` with the exit status when the process dies first. +Use the race in the Trio exec backend instead of waiting indefinitely +on `IPCServer.wait_for_peer()`. + +Add deterministic Trio tests for handshake-first, death-first, +simultaneous, and waiter-error schedules, controlling readiness with +events rather than sleeps. Exercise the full Trio backend with a fake +dead process to prove pre-publication event registration and exact +failed-startup cleanup. + +## Scope boundary + +This prerequisite prevents pre-connection namespace-entry failures +from hanging their parent. Namespace FD transfer itself remains the +next commit. diff --git a/tests/test_spawning.py b/tests/test_spawning.py index ed3829d0..1573d385 100644 --- a/tests/test_spawning.py +++ b/tests/test_spawning.py @@ -10,15 +10,25 @@ API design. """ from functools import partial +from types import SimpleNamespace from typing import ( Any, ) +from unittest.mock import ( + AsyncMock, + MagicMock, +) import pytest import trio import tractor +from tractor._exceptions import ActorFailure from tractor._testing import tractor_test +from tractor.spawn import ( + _spawn, + _trio, +) data_to_pass_down = { 'doggy': 10, @@ -26,6 +36,396 @@ data_to_pass_down = { } +def test_peer_handshake_wins_child_boot_race() -> None: + ''' + A connected child must cancel process-death monitoring cleanly. + + Start the death waiter first and hold it at a checkpoint. Let the + fake server then return one peer event and channel. The helper must + preserve the normal handshake result and cancel the losing process + waiter before its nursery exits. The fake explicitly catches and + re-raises `trio.Cancelled` to prove cancellation caused its exit. + + ''' + async def main() -> None: + ''' + Control the handshake-first schedule with Trio events. + + ''' + uid: tuple[str, str] = ('handshake-child', 'test') + death_started = trio.Event() + death_cancelled = trio.Event() + peer_event = trio.Event() + channel = object() + + async def wait_for_peer( + child_uid: tuple[str, str], + ) -> tuple[trio.Event, object]: + ''' + Return the peer only after death monitoring is active. + + ''' + assert child_uid == uid + await death_started.wait() + return (peer_event, channel) + + async def wait_for_death() -> int: + ''' + Block until the winning handshake cancels this waiter. + + ''' + death_started.set() + try: + await trio.sleep_forever() + except trio.Cancelled: + death_cancelled.set() + raise + + result = await _spawn.wait_for_peer_or_proc_death( + ipc_server=SimpleNamespace( + wait_for_peer=wait_for_peer, + ), + uid=uid, + proc_wait=wait_for_death, + proc_repr='handshake-proc', + ) + + assert result == (peer_event, channel) + assert death_cancelled.is_set() + + trio.run(main) + + +def test_child_death_wins_peer_handshake_race() -> None: + ''' + Pre-handshake child death must fail startup instead of hanging. + + Start the peer waiter first and leave it parked like + `IPCServer.wait_for_peer()` on an unset event. Return a distinctive + process status from the competing waiter, then prove the helper + cancels the handshake and raises `ActorFailure` with child identity, + status, and process diagnostics. + + ''' + async def main() -> None: + ''' + Control the death-first schedule with Trio events. + + ''' + uid: tuple[str, str] = ('dead-child', 'test') + handshake_started = trio.Event() + handshake_cancelled = trio.Event() + + async def wait_for_peer( + child_uid: tuple[str, str], + ) -> tuple[trio.Event, object]: + ''' + Park until process death cancels this handshake waiter. + + ''' + assert child_uid == uid + handshake_started.set() + try: + await trio.sleep_forever() + except trio.Cancelled: + handshake_cancelled.set() + raise + + async def wait_for_death() -> int: + ''' + Report child death after handshake monitoring is active. + + ''' + await handshake_started.wait() + return 23 + + with pytest.raises(ActorFailure) as exc_info: + await _spawn.wait_for_peer_or_proc_death( + ipc_server=SimpleNamespace( + wait_for_peer=wait_for_peer, + ), + uid=uid, + proc_wait=wait_for_death, + proc_repr='dead-proc', + ) + + message: str = str(exc_info.value) + assert repr(uid) in message + assert 'died during boot' in message + assert '(rc=23)' in message + assert 'parent-handshake' in message + assert 'dead-proc' in message + assert handshake_cancelled.is_set() + + trio.run(main) + + +def test_child_death_wins_simultaneous_boot_results() -> None: + ''' + Observed process death must outrank a simultaneous handshake. + + Hold both fake waits behind one barrier with cancellation shielding, + then release them together so both publish a committed result before + sibling cancellation takes effect. Because the child has exited + before receiving its `SpawnSpec`, bootstrap must raise `ActorFailure` + rather than return its briefly established channel. + + ''' + async def main() -> None: + ''' + Release both boot outcomes from one controlled barrier. + + ''' + uid: tuple[str, str] = ('simultaneous-child', 'test') + handshake_ready = trio.Event() + death_ready = trio.Event() + release = trio.Event() + peer_event = trio.Event() + + async def wait_for_peer( + child_uid: tuple[str, str], + ) -> tuple[trio.Event, object]: + ''' + Publish a handshake despite sibling cancellation. + + ''' + assert child_uid == uid + handshake_ready.set() + with trio.CancelScope(shield=True): + await release.wait() + return (peer_event, object()) + + async def wait_for_death() -> int: + ''' + Publish process death despite sibling cancellation. + + ''' + death_ready.set() + with trio.CancelScope(shield=True): + await release.wait() + return 0 + + async def release_both() -> None: + ''' + Open the barrier only after both waiters are parked. + + ''' + await handshake_ready.wait() + await death_ready.wait() + release.set() + + async with trio.open_nursery() as nursery: + nursery.start_soon(release_both) + with pytest.raises( + ActorFailure, + match=r'simultaneous-child.*rc=0', + ): + await _spawn.wait_for_peer_or_proc_death( + ipc_server=SimpleNamespace( + wait_for_peer=wait_for_peer, + ), + uid=uid, + proc_wait=wait_for_death, + ) + + trio.run(main) + + +@pytest.mark.parametrize('failing_waiter', ('handshake', 'death')) +def test_child_boot_race_preserves_waiter_error( + failing_waiter: str, +) -> None: + ''' + Waiter failures must retain their original exception identity. + + Park the non-failing sibling and raise one unique error from either + the peer or process waiter. The helper's internal nursery must + cancel the sibling and re-raise that exact exception instead of + wrapping it in an `ExceptionGroup`. + + ''' + async def main() -> None: + ''' + Trigger one selected waiter after its sibling starts. + + ''' + uid: tuple[str, str] = ('errored-child', 'test') + sibling_started = trio.Event() + wait_error = RuntimeError(f'{failing_waiter} failed') + + async def wait_for_peer( + child_uid: tuple[str, str], + ) -> tuple[trio.Event, object]: + ''' + Raise or park according to the selected peer schedule. + + ''' + assert child_uid == uid + if failing_waiter == 'handshake': + await sibling_started.wait() + raise wait_error + + sibling_started.set() + await trio.sleep_forever() + + async def wait_for_death() -> int: + ''' + Raise or park according to the selected process schedule. + + ''' + if failing_waiter == 'death': + await sibling_started.wait() + raise wait_error + + sibling_started.set() + await trio.sleep_forever() + + with pytest.raises(RuntimeError) as exc_info: + await _spawn.wait_for_peer_or_proc_death( + ipc_server=SimpleNamespace( + wait_for_peer=wait_for_peer, + ), + uid=uid, + proc_wait=wait_for_death, + ) + + assert exc_info.value is wait_error + + trio.run(main) + + +def test_trio_proc_cleans_failed_child_peer_event( + monkeypatch: pytest.MonkeyPatch, +) -> None: + ''' + Death-first Trio startup must release provisional peer state. + + Return one already-dead fake process while its server handshake + parks forever. The fake nursery proves the peer event exists before + provisional child publication. After `ActorFailure`, both that + exact event and the provisional child record must be gone so repeated + failed spawns cannot leak server state. + + ''' + uid: tuple[str, str] = ('dead-trio-child', 'test') + proc: trio.Process = MagicMock(spec=trio.Process) + proc.pid = 1234 + proc.wait = AsyncMock(return_value=23) + proc.poll.return_value = 23 + proc.__str__.return_value = 'dead-trio-proc' + + class FakeServer: + ''' + Hold the peer registry used during Trio child startup. + + ''' + def __init__(self) -> None: + self._peer_connected: dict[ + tuple[str, str], + trio.Event, + ] = {} + + async def wait_for_peer( + self, + child_uid: tuple[str, str], + ) -> tuple[trio.Event, object]: + ''' + Park like a child that never reaches its handshake. + + ''' + assert child_uid == uid + await trio.sleep_forever() + + server = FakeServer() + + class FakeNursery: + ''' + Track provisional child publication and cleanup. + + ''' + def __init__(self) -> None: + self._actor = SimpleNamespace(ipc_server=server) + self._children: dict[tuple[str, str], tuple] = {} + + def _register_child( + self, + subactor: object, + proc: object, + portal: object|None, + ) -> tuple[trio.Event, trio.Event, bool]: + ''' + Require peer-event registration before child publication. + + ''' + assert uid in server._peer_connected + assert portal is None + self._children[uid] = (subactor, proc, portal) + return (trio.Event(), trio.Event(), False) + + async def fake_open_process( + command: list[str], + **kwargs: object, + ) -> trio.Process: + ''' + Return a process whose death wins the bootstrap race. + + ''' + assert command + return proc + + async def fake_wait_for_debugger(**kwargs: object) -> None: + ''' + Keep hard-reap cleanup deterministic and non-interactive. + + ''' + return None + + monkeypatch.setattr( + _trio.trio.lowlevel, + 'open_process', + fake_open_process, + ) + monkeypatch.setattr( + _trio.debug, + 'maybe_wait_for_debugger', + fake_wait_for_debugger, + ) + + nursery = FakeNursery() + subactor = SimpleNamespace( + aid=tractor.msg.Aid( + name=uid[0], + uuid=uid[1], + ), + loglevel=None, + pformat=lambda: 'dead-trio-child', + ) + + async def main() -> None: + ''' + Run the full Trio backend through death-first cleanup. + + ''' + with pytest.raises( + ActorFailure, + match=r'dead-trio-child.*rc=23', + ): + await _trio.trio_proc( + name=uid[0], + actor_nursery=nursery, + subactor=subactor, + errors={}, + bind_addrs=[], + parent_addr=('127.0.0.1', 1616), + _runtime_vars={}, + ) + + trio.run(main) + + assert server._peer_connected == {} + assert nursery._children == {} + + async def run_same_func_in_child( should_be_root: bool, data: dict, diff --git a/tractor/spawn/_spawn.py b/tractor/spawn/_spawn.py index 80f4579a..e30afec3 100644 --- a/tractor/spawn/_spawn.py +++ b/tractor/spawn/_spawn.py @@ -34,6 +34,7 @@ from typing import ( import trio from trio import TaskStatus +from .._exceptions import ActorFailure from ..devx import debug from tractor.runtime._state import ( _runtime_vars, @@ -50,6 +51,7 @@ from tractor.msg import types as msgtypes if TYPE_CHECKING: from tractor.ipc import ( + _server, Channel, ) from tractor.runtime._supervise import ActorNursery @@ -81,6 +83,101 @@ else: await trio.lowlevel.wait_readable(proc.sentinel) +async def wait_for_peer_or_proc_death( + ipc_server: _server.Server, + uid: tuple[str, str], + proc_wait: Callable[[], Awaitable[int]], + proc_repr: object = '', +) -> tuple[trio.Event, Channel]: + ''' + Race a child handshake against process death during bootstrap. + + A child can exit before connecting to its parent. Waiting only on + `IPCServer.wait_for_peer()` would then park its spawning task and + leave the dead process unreaped. Run both waits in one nursery and + let either completed result cancel its sibling. + + Return the normal peer event and channel when the handshake wins. + Raise `ActorFailure` with the process status when death wins. + + Adapted from goodboy's Claude Code-assisted implementation in + commit `3b0724eba85b4014170ed95773e1e41a60d5c513`. + + ''' + handshake: tuple[trio.Event, Channel]|None = None + handshake_error: BaseException|None = None + returncode: int|None = None + death_error: BaseException|None = None + + async def wait_for_handshake() -> None: + ''' + Publish a connected peer before cancelling the death waiter. + + ''' + nonlocal handshake + nonlocal handshake_error + try: + handshake = await ipc_server.wait_for_peer(uid) + except trio.Cancelled: + if ( + returncode is not None + or + death_error is not None + ): + log.debug( + 'Peer-handshake waiter cancelled after ' + f'process wait completed for {uid!r}' + ) + raise + except BaseException as exc: + handshake_error = exc + nursery.cancel_scope.cancel() + + async def wait_for_death() -> None: + ''' + Publish child exit before cancelling the handshake waiter. + + ''' + nonlocal returncode + nonlocal death_error + try: + returncode = await proc_wait() + except trio.Cancelled: + if ( + handshake is not None + or + handshake_error is not None + ): + log.debug( + 'Process-death waiter cancelled after ' + f'peer wait completed for {uid!r}' + ) + raise + except BaseException as exc: + death_error = exc + nursery.cancel_scope.cancel() + + async with trio.open_nursery() as nursery: + nursery.start_soon(wait_for_handshake) + nursery.start_soon(wait_for_death) + + if handshake_error is not None: + raise handshake_error + if death_error is not None: + raise death_error + + if returncode is not None: + raise ActorFailure( + f'Sub-actor {uid!r} died during boot ' + f'(rc={returncode!r}) before completing ' + f'parent-handshake.\n' + f' proc: {proc_repr}' + ) + + assert handshake is not None + return handshake + + def try_set_start_method( key: SpawnMethodKey diff --git a/tractor/spawn/_trio.py b/tractor/spawn/_trio.py index 447ac04e..07b34b4b 100644 --- a/tractor/spawn/_trio.py +++ b/tractor/spawn/_trio.py @@ -51,6 +51,7 @@ from tractor.msg import ( from ._spawn import ( hard_kill, soft_kill, + wait_for_peer_or_proc_death, ) @@ -81,23 +82,24 @@ async def trio_proc( ) -> None: ''' - Create a new ``Process`` using a "spawn method" as (configured using - ``try_set_start_method()``). + Create a new ``Process`` using a "spawn method" as (configured + using ``try_set_start_method()``). - This routine should be started in a actor runtime task and the logic - here is to be considered the core supervision strategy. + This routine should be started in a actor runtime task and the + logic here is to be considered the core supervision strategy. ''' spawn_cmd = [ sys.executable, "-m", - # Hardcode this (instead of using ``_child.__name__`` to avoid a - # double import warning: https://stackoverflow.com/a/45070583 + # Hardcode this (instead of using ``_child.__name__`` to + # avoid a double import warning: + # https://stackoverflow.com/a/45070583 "tractor._child", # We provide the child's unique identifier on this exec/spawn - # line for debugging purposes when viewing the process tree from - # the OS; it otherwise can be passed via the parent channel if - # we prefer in the future (for privacy). + # line for debugging purposes when viewing the process tree + # from the OS; it otherwise can be passed via the parent + # channel if we prefer in the future (for privacy). "--uid", # TODO, how to pass this over "wire" encodings like # cmdline args? @@ -115,20 +117,34 @@ async def trio_proc( ] # Tell child to run in guest mode on top of ``asyncio`` loop if infect_asyncio: - spawn_cmd.append("--asyncio") + spawn_cmd.append('--asyncio') cancelled_during_spawn: bool = False proc: trio.Process|None = None ipc_server: _server.Server = actor_nursery._actor.ipc_server + peer_event: trio.Event|None = None try: try: - proc: trio.Process = await trio.lowlevel.open_process(spawn_cmd, **proc_kwargs) + proc: trio.Process = await trio.lowlevel.open_process( + spawn_cmd, + **proc_kwargs, + ) log.runtime( f'Started new child subproc\n' f'(>\n' f' |_{proc}\n' ) + # `ActorNursery.cancel()` may inspect this event as soon + # as the provisional child is published below. Register + # the event synchronously before + # `wait_for_peer_or_proc_death()` opens its nursery and + # checkpoints. + peer_event = ipc_server._peer_connected.setdefault( + subactor.aid.uid, + trio.Event(), + ) + # No `Portal` exists until the IPC handshake returns # `chan`. Replace this provisional entry with # `Portal(chan)` below. @@ -152,8 +168,11 @@ async def trio_proc( # wait for actor to spawn and connect back to us # channel should have handshake completed by the # local actor by the time we get a ref to it - event, chan = await ipc_server.wait_for_peer( - subactor.aid.uid + event, chan = await wait_for_peer_or_proc_death( + ipc_server=ipc_server, + uid=subactor.aid.uid, + proc_wait=proc.wait, + proc_repr=proc, ) except trio.Cancelled: @@ -269,21 +288,21 @@ async def trio_proc( # to hold off on relaying SIGINT until that child # is complete. # https://github.com/goodboy/tractor/issues/320 - # -[ ] we need to handle non-root parent-actors specially - # by somehow determining if a child is in debug and then - # avoiding cancel/kill of said child by this - # (intermediary) parent until such a time as the root says - # the pdb lock is released and we are good to tear down - # (our children).. + # -[ ] we need to handle non-root parent-actors + # specially by somehow determining if a child is in + # debug and then avoiding cancel/kill of said child + # by this (intermediary) parent until such a time as + # the root says the pdb lock is released and we are + # good to tear down (our children).. # # -[ ] so maybe something like this where we try to - # acquire the lock and get notified of who has it, - # check that uid against our known children? + # acquire the lock and get notified of who has + # it, check that uid against our known children? # this_uid: tuple[str, str] = current_actor().uid # await debug.acquire_debug_lock(this_uid) if proc.poll() is None: - log.cancel(f"Attempting to hard kill {proc}") + log.cancel(f'Attempting to hard kill {proc}') await hard_kill( proc, # NOTE, pass through so post-SIGKILL we @@ -302,10 +321,19 @@ async def trio_proc( subactor=subactor, ) - log.debug(f"Joined {proc}") + log.debug(f'Joined {proc}') else: log.warning('Nursery cancelled before sub-proc started') + if ( + peer_event is not None + and + ipc_server._peer_connected.get( + subactor.aid.uid, + ) is peer_event + ): + ipc_server._peer_connected.pop(subactor.aid.uid) + if not cancelled_during_spawn: # pop child entry to indicate we no longer managing this # subactor