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`))wkt/to_actor_subpkg
parent
23e26b5b32
commit
5c4859860a
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue