Reap `to_actor.run()` children before return

Give each `ActorNursery` child its own reap request and
completion event. Owned one-shots now wait for process joining and
bookkeeping removal before returning.

Escalate unacknowledged cancellation with `proc.kill()` after an
active debugger releases. Latch nursery-wide teardown for monitors
that finish startup late, and snapshot children before cancellation
checkpoints permit concurrent removal.

Cover immediate managed-nursery cleanup, failed cancel
acknowledgements and late monitor registration across Trio TCP/UDS
and `mp_spawn`.

Caught-during: review remediation
Found-via: `/run-tests` test_late_child_reap_registration_is_released

Review: PR #481 (copilot-pull-request-reviewer)
https://github.com/goodboy/tractor/pull/481#discussion_r3514759131

Prompt-IO: ai/prompt-io/opencode/20260818T031532Z_4151b956_prompt_io.md

(this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`))
wkt/to_actor_subpkg
Gud Boi 2026-08-18 01:58:45 -04:00
parent b69a8667d4
commit 49213d170e
8 changed files with 351 additions and 52 deletions

View File

@ -0,0 +1,43 @@
---
model: openai/gpt-5.6-sol
service: opencode
session: pr481-review-fixes-p1-20260818
timestamp: 2026-08-18T03:15:32Z
git_ref: 4151b956
scope: code
substantive: true
raw_file: 20260818T031532Z_4151b956_prompt_io.raw.md
---
## Prompt
Address the approved review findings on PR #481, but work
iteratively: implement and verify one finding at a time, prepare a
separate `/commit-plan` after each fix, and stop for the human commit
before starting the next finding. Begin with the P1 per-child
lifecycle issue. Also publish the already-approved review findings
against the reviewed PR head before editing.
## Response summary
Published the approved non-approving review at head `4151b956`, then
implemented only the P1 lifecycle fix. Owned one-shot actors now use
a child-specific cancellation and process-reap handshake, including
hard escalation for unacknowledged cancellation and deterministic
bookkeeping removal before `to_actor.run()` returns.
## Files changed
- `tractor/runtime/_supervise.py` - coordinate child-specific cancel
and reap.
- `tractor/spawn/_trio.py` - wait on the Trio child's reap request.
- `tractor/spawn/_mp.py` - wait on the multiprocessing child's reap
request.
- `tractor/spawn/_spawn.py` - publish monitor completion centrally.
- `tractor/to_actor/_api.py` - await owned-child process reaping.
- `tests/test_to_actor.py` - cover cleanup, escalation, and startup
ordering.
## Human edits
None - the generated P1 patch remains uncommitted for human review.

View File

@ -0,0 +1,74 @@
---
model: openai/gpt-5.6-sol
service: opencode
timestamp: 2026-08-18T03:15:32Z
git_ref: 4151b956
diff_cmd: git diff HEAD~1..HEAD
---
Implemented only the P1 lifecycle finding from the approved PR #481
review, preserving the requested one-fix-at-a-time commit boundary.
> `git diff HEAD~1..HEAD -- tractor/runtime/_supervise.py`
Added per-child reap request/completion events to `ActorNursery`, a
shielded child-specific cancel-and-reap operation, late-registration
latching for nursery teardown, and cancellation escalation that waits
for debugger release before using non-ignorable process termination.
The nursery-wide cancellation path snapshots child records before
checkpointing so concurrent one-shot cleanup cannot invalidate its
iteration.
> `git diff HEAD~1..HEAD -- tractor/spawn/_trio.py`
Changed Trio child monitors to wait on their per-child reap requests.
> `git diff HEAD~1..HEAD -- tractor/spawn/_mp.py`
Changed multiprocessing child monitors to wait on their per-child reap
requests.
> `git diff HEAD~1..HEAD -- tractor/spawn/_spawn.py`
Ensured every backend publishes child-reap completion after its process
monitor exits.
> `git diff HEAD~1..HEAD -- tractor/to_actor/_api.py`
Changed owned one-shot cleanup to await child-specific process joining
and bookkeeping removal instead of treating the cancel RPC as reaping.
> `git diff HEAD~1..HEAD -- tests/test_to_actor.py`
Added regressions for immediate caller-managed nursery cleanup, failed
cancel acknowledgement escalation, and child registration after a
latched nursery-wide teardown request.
Verification:
`pytest -q tests/test_to_actor.py tests/test_cancellation.py tests/test_spawning.py tests/discovery/test_multi_program.py`
Result: `46 passed, 1 xfailed, 3 xpassed`.
`pytest -q tests/test_to_actor.py --tpt-proto uds`
Result: `13 passed`.
`pytest -q tests/test_to_actor.py --spawn-backend mp_spawn --tpt-proto tcp`
Result: `13 passed`.
One broad verification run was mistakenly launched in parallel with
the UDS and `mp_spawn` actor suites. It timed out
`test_remote_error_from_caller_nursery`; the node passed immediately
in isolation and the complete broad selection then passed serially.
The failure was classified as concurrent test-session interference,
not accepted as a passing boundary result.
Ruff, Python compilation, and `git diff --check` passed for the changed
boundary. Ruff's existing `_trio.py` F401 finding was reproduced at the
unmodified PR head and excluded from attribution to this patch.
No source files were staged, committed, pushed, or used for review
replies. The previously approved top-level review was published before
the fix at reviewed head `4151b956`.

View File

@ -97,9 +97,14 @@ async def test_spawn_from_caller_nursery(
debug_mode: bool,
):
'''
Pass a caller-managed `an: ActorNursery` for the
spawn; the subactor is still one-shot reaped by the
time the call returns.
Pass a caller-managed `an: ActorNursery` for the spawn.
Previously `to_actor.run()` treated an actor-runtime cancel ack
as process reaping, so the call returned while the child monitor
and its `ActorNursery._children` record remained alive until the
entire nursery exited. The assertion inside the still-open
nursery proves child-process joining and record removal now
complete before the one-shot call returns.
'''
async with tractor.open_nursery() as an:
@ -108,6 +113,74 @@ async def test_spawn_from_caller_nursery(
an=an,
n=10,
) == 11
assert not an._children
@tractor_test
async def test_cancel_ack_failure_hard_reaps_child(
monkeypatch: pytest.MonkeyPatch,
start_method: str,
debug_mode: bool,
):
'''
Escalate a failed cancel acknowledgement and reap the child.
`Portal.cancel_actor()` can return `False` when its transport is
already closed without confirming runtime cancellation. The old
one-shot path ignored that result, released the nursery-wide join
gate and then waited forever for a still-running process. This
test forces that exact result without cancelling the actor, caps
the call to detect the former hang and verifies the child monitor
removes its `ActorNursery._children` record before returning.
'''
async def cancel_without_ack(
portal: tractor.Portal,
timeout: float|None = None,
raise_on_timeout: bool = False,
) -> bool:
assert raise_on_timeout
return False
monkeypatch.setattr(
tractor.Portal,
'cancel_actor',
cancel_without_ack,
)
async with tractor.open_nursery() as an:
with trio.fail_after(5):
assert await to_actor.run(
add_one,
an=an,
n=20,
) == 21
assert not an._children
def test_late_child_reap_registration_is_released():
'''
Preserve a nursery-wide reap request across child startup.
A child monitor can checkpoint while connecting to its parent as
the surrounding `ActorNursery` begins teardown. Previously the
nursery signalled only already-registered child events, so a
monitor registering afterward waited forever. This models that
ordering by publishing the nursery-wide request first and proves
the later per-child event inherits its set state immediately.
'''
an = object.__new__(tractor.ActorNursery)
an._join_procs = trio.Event()
an._child_reap_requests = {}
an._child_reaped = {}
an._join_procs.set()
reap_request, _ = an._register_child_reap(
('late_child', 'uid'),
)
assert reap_request.is_set()
@tractor_test

View File

@ -90,7 +90,7 @@ async def _try_cancel_then_kill(
Sends a graceful actor-runtime cancel-RPC via
`Portal.cancel_actor(raise_on_timeout=True)`. If the bounded-wait
expires before the peer ack's, `ActorTooSlowError` is raised and
we escalate via `proc.terminate()` (SIGTERM) per SC-discipline:
we escalate via `proc.kill()` per SC-discipline:
graceful cancel-req -> bounded wait -> hard-kill
@ -102,11 +102,9 @@ async def _try_cancel_then_kill(
the wider write-up.
'''
# XXX, do NOT escalate to `proc.terminate()` while ANY of
# the following are true — SIGTERM-ing a sub would tear
# down its sub-tree including any descendant proxying
# stdio to/from a REPL-locked actor, clobbering the user's
# debug session:
# XXX, delay hard-kill escalation while any debugger guard
# below is active. Killing the sub immediately would tear down
# its tree and clobber an actor proxying a REPL session:
#
# - `Lock.ctx_in_debug is not None`: most precise — some
# actor in the tree is currently REPL-locked. Set in the
@ -122,7 +120,7 @@ async def _try_cancel_then_kill(
# child.
#
# - `debug_mode_active`: this nursery has at least one
# child started with an explicit `debug_mode=` arg
# child started with an explicit `debug_mode=True` arg
# (`ActorNursery._at_least_one_child_in_debug`). Catches
# the case where root is NOT in debug-mode but a
# nursery-direct child opted in.
@ -132,36 +130,57 @@ async def _try_cancel_then_kill(
# mutated by per-child `debug_mode=True`). ORing covers
# every flavor without false-positively skipping
# legitimate hard-kill paths in non-debug trees.
if (
debug_protected: bool = (
debug.Lock.ctx_in_debug is not None
or
_state._runtime_vars.get('_debug_mode', False)
or
debug_mode_active
):
await portal.cancel_actor()
return
)
try:
await portal.cancel_actor(raise_on_timeout=True)
cancelled: bool = await portal.cancel_actor(
raise_on_timeout=not debug_protected,
)
if not cancelled:
if debug_protected:
await debug.maybe_wait_for_debugger(
child_in_debug=(
debug_mode_active
or
debug.Lock.ctx_in_debug is not None
),
header_msg=(
'Delaying subproc hard-reap while '
'debugger locked..\n'
),
)
peer_id: str = portal.channel.aid.reprol()
raise ActorTooSlowError(
f'Peer {peer_id} disconnected before '
f'acknowledging its `Actor.cancel()` RPC'
)
except ActorTooSlowError as too_slow:
log.error(
f'Cancel-ack TIMED OUT for sub-actor\n'
f' uid: {subactor.aid.reprol()!r}\n'
f' reason: {too_slow}\n'
f'-> escalating to `proc.terminate()` (hard-kill)\n'
f'-> escalating to `proc.kill()` (hard-reap)\n'
)
# XXX, the `subint` backend stores an `int` interp-id in the
# `proc` slot (not a `Process`), so it has no `.terminate()`.
# `proc` slot (not a `Process`), so it has no `.kill()`.
# Guard here so a cancel-ack timeout doesn't `AttributeError`
# once that backend lands; its hard-kill path is a TODO.
if hasattr(proc, 'terminate'):
proc.terminate()
if hasattr(proc, 'kill'):
if proc.poll() is None:
proc.kill()
else:
log.error(
f'Cannot hard-kill sub-actor — backend proc-handle '
f'{proc!r} ({type(proc).__name__!r}) has no '
f'`.terminate()`!\n'
f'`.kill()`!\n'
f' uid: {subactor.aid.reprol()!r}\n'
f'TODO: per-backend cancel-escalation.\n'
)
@ -220,6 +239,14 @@ class ActorNursery:
] = {}
self._join_procs = trio.Event()
self._child_reap_requests: dict[
tuple[str, str],
trio.Event,
] = {}
self._child_reaped: dict[
tuple[str, str],
trio.Event,
] = {}
self._at_least_one_child_in_debug: bool = False
self.errors = errors
self._scope_error: BaseException|None = None
@ -284,6 +311,79 @@ class ActorNursery:
# self._cancelled_caught
)
def _register_child_reap(
self,
uid: tuple[str, str],
) -> tuple[trio.Event, trio.Event]:
'''
Register a child monitor's process-reap events.
'''
reap_request = trio.Event()
reaped = trio.Event()
self._child_reap_requests[uid] = reap_request
self._child_reaped[uid] = reaped
if self._join_procs.is_set():
reap_request.set()
return reap_request, reaped
def _request_reap_all(self) -> None:
'''
Release every child monitor into its process-join phase.
'''
self._join_procs.set()
for reap_request in tuple(
self._child_reap_requests.values()
):
reap_request.set()
def _mark_child_reaped(
self,
uid: tuple[str, str],
) -> None:
'''
Publish completed child-process teardown to its waiter.
'''
self._children.pop(uid, None)
self._child_reap_requests.pop(uid, None)
reaped: trio.Event|None = self._child_reaped.pop(
uid,
None,
)
if reaped is not None:
reaped.set()
async def _cancel_and_reap_child(
self,
portal: Portal,
) -> None:
'''
Cancel, join and unregister one nursery-owned child.
'''
uid: tuple[str, str] = portal.channel.aid.uid
child_entry = self._children.get(uid)
if child_entry is None:
return
subactor, proc, _ = child_entry
reap_request: trio.Event = self._child_reap_requests[uid]
reaped: trio.Event = self._child_reaped[uid]
with trio.CancelScope(shield=True):
try:
await _try_cancel_then_kill(
portal,
proc,
subactor,
self._at_least_one_child_in_debug,
)
finally:
reap_request.set()
await reaped.wait()
async def start_actor(
self,
name: str,
@ -333,7 +433,7 @@ class ActorNursery:
# allow setting debug policy per actor
if debug_mode is not None:
_rtv['_debug_mode'] = debug_mode
self._at_least_one_child_in_debug = True
self._at_least_one_child_in_debug |= debug_mode
enable_modules = list(enable_modules or [])
proc_kwargs = dict(proc_kwargs or {})
@ -484,7 +584,7 @@ class ActorNursery:
# TODO: impl a repr for spawn more compact
# then `._children`..
children: dict = self._children
children: tuple = tuple(self._children.values())
child_count: int = len(children)
msg: str = f'Cancelling actor nursery with {child_count} children\n'
@ -503,7 +603,7 @@ class ActorNursery:
subactor,
proc,
portal,
) in children.values():
) in children:
# TODO: are we ever even going to use this or
# is the spawning backend responsible for such
@ -523,7 +623,9 @@ class ActorNursery:
await event.wait()
# channel/portal should now be up
_, _, portal = children[subactor.aid.uid]
_, _, portal = self._children[
subactor.aid.uid
]
# XXX should be impossible to get here
# unless method was called from within
@ -570,14 +672,14 @@ class ActorNursery:
subactor,
proc,
portal,
) in children.values():
) in children:
log.warning(f"Hard killing process {proc}")
proc.terminate()
else:
self._cancelled_caught
# mark ourselves as having (tried to have) cancelled all subactors
self._join_procs.set()
self._request_reap_all()
@acm
@ -639,7 +741,7 @@ async def _open_and_supervise_one_cancels_all_nursery(
'Waiting on subactors to complete:\n'
f'>}} {len(an._children)}\n'
)
an._join_procs.set()
an._request_reap_all()
except BaseException as _inner_err:
inner_err = _inner_err
@ -658,7 +760,7 @@ async def _open_and_supervise_one_cancels_all_nursery(
# if the caller's scope errored then we activate our
# one-cancels-all supervisor strategy (don't
# worry more are coming).
an._join_procs.set()
an._request_reap_all()
# XXX NOTE XXX: hypothetically an error could
# be raised and then a cancel signal shows up

View File

@ -170,13 +170,16 @@ async def mp_proc(
# any process we may have started.
portal = Portal(chan)
reap_request, _ = actor_nursery._register_child_reap(
subactor.aid.uid,
)
actor_nursery._children[subactor.aid.uid] = (subactor, proc, portal)
# unblock parent task
task_status.started(portal)
# wait for ``ActorNursery`` block to signal that
# subprocesses can be waited upon.
# wait for this child or its `ActorNursery` to signal that
# the subprocess can be joined.
# This is required to ensure synchronization
# with user code that may want to manually await results
# from nursery spawned sub-actors. We don't want the
@ -185,7 +188,7 @@ async def mp_proc(
# nursery block closes do we allow subactor results to be
# awaited and reported upwards to the supervisor.
with trio.CancelScope(shield=True):
await actor_nursery._join_procs.wait()
await reap_request.wait()
async with trio.open_nursery() as nursery:
if portal in actor_nursery._cancel_after_result_on_exit:

View File

@ -446,18 +446,21 @@ async def new_proc(
# mark the new actor with the global spawn method
subactor._spawn_method = _spawn_method
await target(
name,
actor_nursery,
subactor,
errors,
bind_addrs,
parent_addr,
_runtime_vars, # run time vars
infect_asyncio=infect_asyncio,
task_status=task_status,
proc_kwargs=proc_kwargs
)
try:
await target(
name,
actor_nursery,
subactor,
errors,
bind_addrs,
parent_addr,
_runtime_vars, # run time vars
infect_asyncio=infect_asyncio,
task_status=task_status,
proc_kwargs=proc_kwargs
)
finally:
actor_nursery._mark_child_reaped(subactor.aid.uid)
# NOTE: bottom-of-module to avoid a circular import since the

View File

@ -161,6 +161,9 @@ async def trio_proc(
assert proc
portal = Portal(chan)
reap_request, _ = actor_nursery._register_child_reap(
subactor.aid.uid,
)
actor_nursery._children[subactor.aid.uid] = (
subactor,
proc,
@ -191,9 +194,10 @@ async def trio_proc(
# resume caller at next checkpoint now that child is up
task_status.started(portal)
# wait for ActorNursery.wait() to be called
# wait for this child or its `ActorNursery` to request
# process joining.
with trio.CancelScope(shield=True):
await actor_nursery._join_procs.wait()
await reap_request.wait()
async with trio.open_nursery() as nursery:
if portal in actor_nursery._cancel_after_result_on_exit:

View File

@ -103,13 +103,10 @@ async def _invoke_in_subactor(
**fn_kwargs,
)
finally:
# one-shot semantics: the subactor's lifetime is
# bound to its lone task's completion; the
# cancel-req's bounded wait is shielded
# internally (see `Portal.cancel_actor()`) so
# this reap also runs when the caller's scope
# was itself cancelled.
await portal.cancel_actor()
# Cancel and join this child before returning. The nursery
# helper shields teardown, escalates a missed cancel ack and
# waits for the child monitor to remove its process record.
await an._cancel_and_reap_child(portal)
async def run(