From 5c4859860a4a0ff686e9ccd724386d1c1a2c2df4 Mon Sep 17 00:00:00 2001 From: goodboy Date: Fri, 21 Aug 2026 01:20:08 -0400 Subject: [PATCH] Close late `ActorNursery` registration race A child could pass the early `.start_actor()` guard, miss the `.cancel()` child snapshot and register afterward. Its monitor inherited a reap request without runtime cancellation and could wait forever. - publish child/reap events before sampling `_cancel_called` - make MP abort before `proc.start()` when cancellation won - kill a Trio child opened after cancellation won registration - reject starts begun after nursery cancellation is already visible - add deterministic registration and MP no-start regressions - drop the touched Trio backend's stale `get_runtime_vars` import Prompt-IO: ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.md (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`)) --- .../20260821T040803Z_3c1bbe73_prompt_io.md | 57 +++++++++ ...20260821T040803Z_3c1bbe73_prompt_io.raw.md | 48 +++++++ tests/test_to_actor.py | 121 ++++++++++++++++++ tractor/runtime/_supervise.py | 29 +++++ tractor/spawn/_mp.py | 25 ++-- tractor/spawn/_trio.py | 21 ++- 6 files changed, 288 insertions(+), 13 deletions(-) create mode 100644 ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.md create mode 100644 ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.raw.md diff --git a/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.md b/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.md new file mode 100644 index 00000000..ff0e98f6 --- /dev/null +++ b/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.md @@ -0,0 +1,57 @@ +--- +model: openai/gpt-5.6-sol +service: opencode +session: 76c5d31c-5a2f-4503-9b16-410ee7f4fab3 +timestamp: 2026-08-21T04:08:03Z +git_ref: 3c1bbe73 +scope: code +substantive: true +raw_file: 20260821T040803Z_3c1bbe73_prompt_io.raw.md +--- + +## Prompt + +Continue PR #481 review cleanup one item at a time. Compare the +remaining child-registration/reaping experiment against PR #484, +identify the next valid behavior, and generate an exact commit plan for +that boundary without committing automatically. + +## Response summary + +Separated late-child admission from the broader bounded-reap rewrite. +Added one synchronous `ActorNursery._register_child()` publication step +that stores the child and its reap events before sampling nursery +cancellation. This closes both scheduler orderings around +`ActorNursery.cancel()`'s child snapshot. + +The MP backend registers immediately before synchronous process startup +and refuses to start when cancellation already owns the child. The Trio +backend registers immediately after `open_process()` and kills that +already-created process when registration observes cancellation. An +early `start_actor()` guard rejects calls begun after cancellation is +already visible. + +Deterministic tests cover the nursery registration ordering and the MP +no-start invariant. Comparison with PR #484 confirmed that its retained +generic nursery/backends do not close this race. + +## Files changed + +- `tractor/runtime/_supervise.py` - atomically publish child ownership + and reject actor starts after nursery cancellation. +- `tractor/spawn/_mp.py` - register before synchronous process startup + and abort a cancellation-owned child. +- `tractor/spawn/_trio.py` - register immediately after process creation, + kill a cancellation-owned child, and remove its stale unused import. +- `tests/test_to_actor.py` - cover late registration and MP startup + suppression. + +## Human edits + +The human required review extras to be handled one item and one +behavioral commit at a time, with each item compared against PR #484 +before acceptance. That direction split this late-registration fix from +the original broad experiment's bounded post-ack reaping, +`ActorNursery.cancel()` hard-reap rewrite, and debugger/error behavior. +The human accepted the narrower late-registration boundary by requesting +its commit plan. No direct source-line edits were made by the human. diff --git a/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.raw.md b/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.raw.md new file mode 100644 index 00000000..ad9917ca --- /dev/null +++ b/ai/prompt-io/opencode/20260821T040803Z_3c1bbe73_prompt_io.raw.md @@ -0,0 +1,48 @@ +--- +model: openai/gpt-5.6-sol +service: opencode +timestamp: 2026-08-21T04:08:03Z +git_ref: 3c1bbe73 +diff_cmd: git diff HEAD~1..HEAD +--- + +Compare the remaining child-registration and reaping experiment with +PR #484, then identify the next review item without changing code. + +The next item is the late-child admission race. A spawn can pass +`ActorNursery.start_actor()`'s early cancellation check, then be absent +from `ActorNursery.cancel()`'s child snapshot and register afterward. +The existing reap-request latch releases its monitor but does not send +runtime cancellation, so the monitor can wait forever for a still-live +process. + +> `git diff HEAD~1..HEAD -- tractor/runtime/_supervise.py` + +`ActorNursery._register_child()` publishes the child, installs its reap +events, and samples `ActorNursery._cancel_called` without a checkpoint. +The two scheduler orderings are then complete: registration first puts +the child in the cancel snapshot, while cancellation first makes the +backend abort the late registration. + +> `git diff HEAD~1..HEAD -- tractor/spawn/_mp.py` + +The multiprocessing backend registers immediately before `proc.start()` +and refuses to start a process already owned by nursery cancellation. +There is no Trio checkpoint between registration and process startup. + +> `git diff HEAD~1..HEAD -- tractor/spawn/_trio.py` + +The Trio backend registers immediately after `open_process()` and kills +the newly opened process if cancellation won the registration race. Its +stale unused `get_runtime_vars` import is removed so the touched module +remains lint-clean. + +> `git diff HEAD~1..HEAD -- tests/test_to_actor.py` + +Deterministic regressions prove late registration observes cancellation +and that the MP backend never starts a process after cancellation owns +its registration. + +PR #484 retains the affected generic nursery and spawn-backend paths and +does not close this race. Keep this fix in PR #481 as its own commit; +review bounded post-`CancelAck` reaping separately. diff --git a/tests/test_to_actor.py b/tests/test_to_actor.py index 28443d06..a4b18f9c 100644 --- a/tests/test_to_actor.py +++ b/tests/test_to_actor.py @@ -8,6 +8,7 @@ https://github.com/goodboy/tractor/issues/477 ''' from functools import partial from pathlib import Path +from types import SimpleNamespace import pytest import trio @@ -21,6 +22,7 @@ from tractor._testing import tractor_test from tractor._exceptions import ActorTooSlowError from tractor.msg import ptr as msgptr from tractor.msg.ptr import NamespacePath +from tractor.spawn import _mp as mp_spawn from tractor.to_actor import _api as to_actor_api @@ -317,6 +319,125 @@ def test_cancel_actor_timeout_closes_blocked_send(): ) +def _mock_actor_nursery() -> tractor.ActorNursery: + an = object.__new__(tractor.ActorNursery) + an._children = {} + an._join_procs = trio.Event() + an._child_reap_requests = {} + an._child_reaped = {} + an._at_least_one_child_in_debug = False + an._cancel_called = False + return an + + +def test_late_child_registration_observes_cancel(): + ''' + Make registration atomically observe nursery cancellation. + + `ActorNursery.cancel()` previously snapshotted `_children` before + its next checkpoint. A process monitor registering after that + snapshot received a reap request but no runtime cancellation, then + waited forever for natural exit. Publishing the child and its reap + events together returns cancellation ownership to the late monitor. + + ''' + an = _mock_actor_nursery() + an._cancel_called = True + aid = tractor.msg.Aid( + name='late_child', + uuid='test', + ) + subactor = SimpleNamespace(aid=aid) + proc = object() + + ( + reap_request, + reaped, + cancel_during_registration, + ) = an._register_child( + subactor, + proc, + None, + ) + + assert cancel_during_registration + assert an._children[aid.uid] == ( + subactor, + proc, + None, + ) + assert an._child_reap_requests[aid.uid] is reap_request + assert an._child_reaped[aid.uid] is reaped + + +def test_mp_late_registration_never_starts_process( + monkeypatch: pytest.MonkeyPatch, +): + ''' + Refuse to start an MP child already owned by nursery cancellation. + + A concurrent `ActorNursery.cancel()` can publish cancellation after + `start_actor()` checks its flag but before the MP backend registers + its process. The fake registration reports that exact schedule. + Proving `FakeProcess.start()` is never called prevents a child from + starting after it was omitted from the cancellation snapshot. + + ''' + class FakeProcess: + started: bool = False + + def start(self) -> None: + self.started = True + + process = FakeProcess() + + class FakeContext: + def get_start_method(self) -> str: + return 'spawn' + + def Process(self, **kwargs: object) -> FakeProcess: + assert kwargs + return process + + nursery = SimpleNamespace( + _register_child=lambda *args: ( + trio.Event(), + trio.Event(), + True, + ), + ) + subactor = SimpleNamespace( + aid=tractor.msg.Aid( + name='late_mp_child', + uuid='test', + ), + ) + monkeypatch.setattr( + mp_spawn._spawn, + '_ctx', + FakeContext(), + ) + + with pytest.raises( + RuntimeError, + match='nursery began cancelling', + ): + trio.run( + partial( + mp_spawn.mp_proc, + name='late_mp_child', + actor_nursery=nursery, + subactor=subactor, + errors={}, + bind_addrs=[], + parent_addr=SimpleNamespace(), + _runtime_vars={}, + ) + ) + + assert not process.started + + def test_late_child_reap_registration_is_released(): ''' Preserve a nursery-wide reap request across child startup. diff --git a/tractor/runtime/_supervise.py b/tractor/runtime/_supervise.py index 366bb61d..bc8df672 100644 --- a/tractor/runtime/_supervise.py +++ b/tractor/runtime/_supervise.py @@ -327,6 +327,29 @@ class ActorNursery: reap_request.set() return reap_request, reaped + def _register_child( + self, + subactor: Actor, + proc: 'ProcessType', + portal: Portal|None, + ) -> tuple[trio.Event, trio.Event, bool]: + ''' + Atomically publish one child and its reap coordination. + + ''' + uid: tuple[str, str] = subactor.aid.uid + self._children[uid] = ( + subactor, + proc, + portal, + ) + reap_request, reaped = self._register_child_reap(uid) + return ( + reap_request, + reaped, + self._cancel_called, + ) + def _request_reap_all(self) -> None: ''' Release every child monitor into its process-join phase. @@ -419,6 +442,12 @@ class ActorNursery: ''' __runtimeframe__: int = 1 # noqa + if self._cancel_called: + raise RuntimeError( + 'Cannot start an actor in a cancelling ' + '`ActorNursery`' + ) + loglevel: str = ( loglevel or self._actor.loglevel diff --git a/tractor/spawn/_mp.py b/tractor/spawn/_mp.py index ee56b770..593d4e75 100644 --- a/tractor/spawn/_mp.py +++ b/tractor/spawn/_mp.py @@ -138,12 +138,22 @@ async def mp_proc( # daemon=True, name=name, ) - - # `multiprocessing` only (since no async interface): - # register the process before start in case we get a cancel - # request before the actor has fully spawned - then we can wait - # for it to fully come up before sending a cancel request - actor_nursery._children[subactor.aid.uid] = (subactor, proc, None) + # `multiprocessing` only (since no async interface): publish the + # process and its reap coordination before start so cancellation + # can own every subsequently started child. + ( + reap_request, + _, + cancel_during_registration, + ) = actor_nursery._register_child( + subactor, + proc, + None, + ) + if cancel_during_registration: + raise RuntimeError( + 'Actor registered after its nursery began cancelling' + ) proc.start() if not proc.is_alive(): @@ -170,9 +180,6 @@ 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 diff --git a/tractor/spawn/_trio.py b/tractor/spawn/_trio.py index a2f38b71..d7f49d94 100644 --- a/tractor/spawn/_trio.py +++ b/tractor/spawn/_trio.py @@ -39,7 +39,6 @@ from tractor.runtime._state import ( current_actor, is_root_process, debug_mode, - get_runtime_vars, ) from tractor.log import get_logger from tractor.discovery._addr import UnwrappedAddress @@ -131,6 +130,23 @@ async def trio_proc( f' |_{proc}\n' ) + ( + reap_request, + _, + cancel_during_registration, + ) = actor_nursery._register_child( + subactor, + proc, + None, + ) + if cancel_during_registration: + cancelled_during_spawn = True + proc.kill() + raise RuntimeError( + 'Actor registered after its nursery began ' + 'cancelling' + ) + # 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 @@ -161,9 +177,6 @@ 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,