Using `.get_stream_addrs()` such that we always (*can*) assign the peer
end's PID in the `._raddr`.
Also factor common `ConnectionError` re-raising into
a `_reraise_as_connerr()`-@cm.
Such that the caller can be responsible for their own (nursery) scoping
as needed and, for the latter fn's case with
a `trio.Nursery.CancelStatus.encloses()` check to ensure the `tn` is
a valid parent-ish.
Some deats,
- in `gather_contexts()`, mv the `try/finally` outside the nursery block
to ensure we always do the `parent_exit`.
- for `maybe_open_context()` we do a naive task-tree hierarchy audit to
ensure the provided scope is not *too* child-ish (with what APIs `trio`
gives us, see above), OW go with the old approach of using the actor's
private service nursery.
Also,
* better report `trio.Cancelled` around the cache-miss `yield`
cases and ensure we **never** unmask triggering key-errors.
* report on any stale-state with the mutex in the `finally` block.
Since the `await service_n.start()` on key-err can be cancel-masked
(checkpoint interrupted before `_Cache.run_ctx` completes), we need to
always `lock.release()` in to avoid lock-owner-state corruption and/or
inf-hangs in peer cache-hitting tasks.
Deats,
- add a `try/except/finally` around the key-err triggered cache-miss
`service_n.start(_Cache.run_ctx, ..)` call, reporting on any taskc
and always `finally` unlocking.
- fill out some log msg content and use `.debug()` level.
As mentioned in prior testing commit, it can cause the worst kind of
hangs, the SIGINT ignoring kind.. Pretty sure there was never any reason
outside some esoteric multi-actor debugging case, and pretty sure that
already was solved?
Namely `test_empty_mngrs_input_raises()` was failing due to
lazy-iterator use as input to `mngrs` which i guess i added support for
a while back (by it doing a `list(mngrs)` internally)? So just change it
to `gather_contexts(mngrs=())` and also tweak the `trio.fail_after(3)`
since it appears that the prior 1sec was causing
too-fast-of-a-cancellation (before the cluster fully spawned) and thus
the expected `ValueError` never to show..
Also, mask the `tractor.trionics.collapse_eg()` usage (again?) in
`open_actor_cluster()` since it seems unnecessary.
Seems that the way the actor-nursery interacts with the
`.trionics.gather_contexts()` API on cancellation makes our
`.trionics.collapse_eg()` not work as intended?
I need to dig into how `ActorNursery.cancel()` and `.__aexit__()` might
be causing this discrepancy..
Consider this a commit-of-my-index type save for rn.
It was originally this way; I forgot to flip it back when discarding the
`except*` handler impl..
Specially handle the `exc.__cause__` case where we raise from any
detected underlying cause and OW `from None` to suppress the eg's tb.
Since it turns out the semantics are basically inverse of normal
`except` (particularly for re-raising) which is hard to get right, and
bc it's a lot easier to just delegate to what `trio` already has behind
the `strict_exception_groups=False` setting, Bp
I added a rant here which will get removed shortly likely, but i think
going forward recommending against use of `except*` is prudent for
anything low level enough in the runtime (like trying to filter begs).
Dirty deats,
- copy `trio._core._run.collapse_exception_group()` to here with only
a slight mod to remove the notes check and tb concatting for the
collapse case.
- rename `maybe_collapse_eg()` - > `get_collapsed_eg()` and delegate it
directly to the former `trio` fn; return `None` when it returns the
same beg without collapse.
- simplify our own `collapse_eg()` to either raise the collapsed `exc`
or original `beg`.
I dunno what exactly I was thinking but we definitely don't want to
**ever** raise from the original exc-group, instead always raise from
any original `.__cause__` to be consistent with the embedded src-error's
context.
Also, adjust `maybe_collapse_eg()` to return `False` in the non-single
`.exceptions` case, again don't know what I was trying to do but this
simplifies caller logic and the prior return-semantic had no real
value..
This fixes some final usage in the runtime (namely top level nursery
usage in `._root`/`._runtime`) which was previously causing test suite
failures prior to this fix.
Seems to cause the following test suites to fail however..
- 'test_advanced_faults.py::test_ipc_channel_break_during_stream'
- 'test_advanced_faults.py::test_ipc_channel_break_during_stream'
- 'test_clustering.py::test_empty_mngrs_input_raises'
Also tweak some ctxc request logging content.
That is just throughout the core library, not the tests yet. Again, we
simply change over to using our (nearly equivalent?)
`.trionics.collapse_eg()` in place of the already deprecated
`strict_exception_groups=False` flag in the following internals,
- the conc-fan-out tn use in `._discovery.find_actor()`.
- `._portal.open_portal()`'s internal tn used to spawn a bg rpc-msg-loop
task.
- the daemon and "run-in-actor" layered tn pair allocated in
`._supervise._open_and_supervise_one_cancels_all_nursery()`.
The remaining loose-eg usage in `._root` and `._runtime` seem to be
necessary to keep the test suite green?? For the moment these are left
out.
- drop the (never/un)used `.get_chans()`.
- add #TODO for factoring many methods into a new `.rpc`-subsys/pkg
primitive, like an `RPCMngr/Server` type eventually.
- add todo to maybe mv `.get_parent()` elsewhere?
- move masked `._hard_mofo_kill()` to bottom.
Namely inside various bootup-sequences in `._root` and `._runtime`
particularly in the root actor to support both better tpt-address
denoting in our logging and as part of clarifying logic around setting
the root's registry addresses which is soon to be much better factored
out of the core and into an explicit subsystem + API.
Some `_root.open_root_actor()` deats,
- set `registry_addrs` to a new `uw_reg_addrs` (uw: unwrapped) to be
more explicit about wrapped addr types thoughout.
- instead ensure `registry_addrs` are the wrapped types and pass down
into the root `Actor` singleton-instance.
- factor the root-actor check + rt-vars update (updating the `'_root_addrs'`)
out of `._runtime.async_main()` into this fn.
- as previous, set `trans_bind_addrs = uw_reg_addrs` in unwrapped form since it will
be passed down both through rt-vars as `'_root_addrs'` and to
`._runtim.async_main()` as `accept_addrs` (which is then passed to the
IPC server).
- adjust/simplify much logging.
- shield the `await actor.cancel(None) # self cancel` to avoid any
finally-footguns.
- as mentioned convert the
For `_runtime.async_main()` tweaks,
- expect `registry_addrs: list[Address]|None = None` with appropriate
unwrapping prior to setting both `.reg_addrs` and the equiv rt-var.
- add a new `.registry_addrs` prop for the wrapped form.
- convert a final loose-eg for the `service_nursery` to use
`collapse_eg()`.
- simplify teardown report logging.
Mostly just multi-line code styling again: always putting standalone
`'f\n'` on separate LOC so it reads like it renders to console. Oh and
and a level drop to `.runtime()` for rx-msg reports.
Again using `Channel.aid.reprol()`, `.devx.pformat.nest_from_op()` and
converting to multi-line code style an ' for str-report-contents. Tweak
some imports to sub-mod level as well.
Again for nicer console logging. Also fix a double `req_chan` arg bug
when passed to `_invoke` in the `self.cancel()` rt-ep; don't update the
`kwargs: dict` just merge in `req_chan` input at call time.
Including a new one-line `_shutdown_msg: str` which we mod-var-set for
testing usage and some denoising at `.info()` level. Adjust `Actor()`
instantiating input to the new `.registry_addrs` wrapped addrs property.
Mostly adjusting indentation, noise level, and clarity via `.pformat()`
tweaks more general use of `.devx.pformat.nest_from_op()`.
Specific impl deats,
- use `pformat.ppfmt()/`nest_from_op()` more seriously throughout
`._server`.
- add a `._server.Endpoint.pformat()`.
- add `._server.Server.len_peers()` and `.repr_state()`.
- polish `Server.pformat()`.
- drop some redundant `log.runtime()`s from `._serve_ipc_eps()` instead
leaving-them-only/putting-them in the caller pub meth.
- `._tcp.start_listener()` log the bound addr, not the input (which may
be the 0-port.
- only generate a repr in `.from_addr()` when log level is >= 'runtime'.
|_ add a todo about supporting this optimization more generally on our
adapter.
- fix `Channel.pformat()` to show unknown peer field line fmt correctly.
- add a `Channel.maddr: str` which just delegates directly to the
`._transport` like other pass-thru property fields.
To start trying out,
- using in the `Start`-msg handler-block to repr the msg coming
*from* a `repr(Channel)` using '<=)` sclang op.
- for a completed RPC task in `_invoke_non_context()`.
- for the msg loop task's termination report.
Such as the `MsgTransport.stream` and `.drain` attrs since they're
rarely that important at the chan level. Also start adopting
a `.<attr>=` style for actual attrs of the type versus a `<name>:
` style for meta-field info lines.
To simplify `.pformat()` output when the new `privates: bool` is unset
(the default) this adds new public attrs to wrap an actor's
cancellation status as well as provide a `.repr_state: str` (similar to
our equiv on `Context`). Rework `.pformat()` to render a much simplified
repr using all these new refinements.
Further, port the `.cancel()` method to use `.msg.types.Aid` for all
internal `requesting_uid` refs (now renamed with `_aid`) and in all
called downstream methods.
New cancel-state iface deats,
- rename `._cancel_called_by_remote` -> `._cancel_called_by` and expect
it to be set as an `Aid`.
- add `.cancel_complete: bool` which flags whether `.cancel()` ran to
completion.
- add `.cancel_called: bool` which just wraps `._cancel_called` (and
which likely will just be dropped since we already have
`._cancel_called_by`).
- add `.cancel_caller: Aid|None` which wraps `._cancel_called_by`.
In terms of using `Aid` in cancel methods,
- rename vars with `_aid` suffix in `.cancel()` (and wherever else).
- change `.cancel_rpc_tasks()` input param to `req_aid: msgtypes.Aid`.
- do the same for `._cancel_task()` and (for now until we adjust its
internals as well) use the `Aid.uid` remap property when assigning
`Context._canceller`.
- adjust all log msg refs to match obvi.
Providing the legacy `.uid -> tuple` style id (since still used for the
`Actor._contexts` table) and a `repr-one-line` method `.reprol() -> str`
for rendering a compact unique actor ID summary (useful in
logging/.pformat()s at the least).
Since it's for beg filtering, the current impl should be renamed anyway;
it's not just for filtering cancelled excs.
Deats,
- added a real doc string, links to official eg docs and fixed the
return typing.
- adjust all internal imports to match.
To resolve the recently added and failing
`test_remote_exc_relay::test_unmasked_remote_exc`: never allow
`trio.Cancelled` to mask an underlying user-code exception, ever.
Our first real-world (runtime internal) use case for the new
`.trionics.maybe_raise_from_masking_exc()` such that the failing
test now passes with an properly relayed remote RTE unmasking B)
Details,
- flip the `Context._scope_nursery` to the default strict-eg behaviour
and instead stack its outer scope with a `.trionics.collapse_eg()`.
- wrap the inner-most scope (after `msgops.maybe_limit_plds()`) with
a `maybe_raise_from_masking_exc()` to ensure user-code errors are
never masked by `trio.Cancelled`s.
Some err-reporting refinement,
- always capture any `scope_err` from the entire block for debug
purposes; report it in the `finally` block's log.
- always capture any suppressed `maybe_re`, output from
`ctx.maybe_raise()`, and `log.cancel()` report it.
To handle captured non-egs (when the now optional `tn` isn't provided)
as well as yield up a `BoxedMaybeException` which contains any detected
and un-masked `exc_ctx` as its `.value`.
Also add some additional tooling,
- a `raise_unmasked: bool` toggle for when the caller just wants to
report the masked exc and not raise-it-in-place of the masker.
- `extra_note: str` which by default is tuned to the default
`unmask_from = (trio.Cancelled,)` but which can be used to deliver
custom exception msg content.
- `always_warn_on: tuple[BaseException]` which will always emit
a warning log of what would have been the raised-in-place-of
`ctx_exc`'s msg for special cases where you want to report
a masking case that might not be otherwise noticed by the runtime
(cough like a `Cancelled` masking another `Cancelled) but which
you'd still like to warn the caller about.
- factor out the masked-`ext_ctx` predicate logic into
a `find_masked_excs()` and also use it for non-eg cases.
Still maybe todo?
- rewrapping multiple masked sub-excs in an eg back into an eg? left in
#TODOs and a pause-point where applicable.
Turns out there were some subtle internal bugs discovered by the just
added `tests/devx/test_tooling::test_crash_handler_cms` suite.
So this fixes,
- a mis-ordering around `rt_repl_fixture :=` in the logic of
`DebugStatus.maybe_enter_repl_fixture()`.
- `.devx.debug._post_mortem._post_mortem()` ensuring we now **always**
call `DebugStatus.release()`, and thus unwind the exist-stack managing
the `repl_fixture` exit/teardown, **even in the case** where
`yield False` is delivered from the user-fixture-fn (meaning
`dnter_repl=False`) thus triggering an early `return` (as is done in
the new test suite).
Opting for performance over broad multi-actor "debug-ability" from
sync-function-contexts when `debug_mode=True` is set;
IOW prefer no behind-the-scenes `greenlet` perf impact over being
able to use an actor-safe `breakpoint()` wherever as per,
https://greenback.readthedocs.io/en/latest/principle.html#performance
Adjust the breakpoint restore ex script to match.
Dropping the `_maybe_open_repl_fixture()` approach and instead using
a `DebugStatus._fixture_stack = ExitStack()` which provides for much
simpler support around both sync and async pausing APIs thanks to only
invoking `repl_fixture.__exit__()` on actual `PdbREPL` interaction being
complete!
Deats,
- all `repl_fixture` detection logic still happens in one place (the new
method) but we aren't limited to closing it via an immediate post REPL
`.__exit__()` call which instead is triggered by,
- `DebugStatus.release()` which now calls `._fixture_stack.close()` and
thus only invokes `repl_fixture.__exit__()` when user REPL-ing is
**actually complete** an arbitrary amount of debugging time later.
- include the notes for `@acm` support above the new method, though not
sure if they're as relevant any more?
Benefits,
- we can drop the previously added indent levels from
`_enter_repl_sync()` and `_post_mortem()`.
- now we automatically have support for the `.pause_from_sync()` API
since `_enter_repl_sync()` doesn't close the prior
`_maybe_open_repl_fixture()` immediately when `debug_func=None`; the
user's `__exit__()` is only ever called once `.release()` is.
Other,
- add big 'CASE' comments around the various blocks in
`.pause_from_sync()`, i was having trouble figuring out which i was
using from a `breakpoint()` in a dependent app..
Which cleans out the pkg-mod to just the expected exports with (its
longstanding todo comment list) and thus a separation-of-concerns
and smaller mod-file sizes via the following new sub-mods:
- `._trace` for the `.pause()`/`breakpoint()`/`pdb.set_trace()`-style
APIs including all sync-caller variants.
- `._post_mortem` to contain our async `.post_mortem()` and all other
public crash handling APIs for use from sync callers.
- `._sync` for the high-level syncing helper-routines used throughout the
runtime to avoid multi-proc TTY use collisions.
And also,
- remove `hide_runtime_frames()` since moved to `.devx._frame_stack`.
Orig commit was,
"9c0de24 Be explicit with `SpawnSpec` processing in subs"
The commit was picked onto an upstream branch but at that time there was
no `.devx.debug` subpkg yet, hence this revert to the original patch's
module path.
From what was originall the `.devx._debug` monolith module, since that
file was way out of ctl in terms of LoC!
New modules so far include,
- ._repl: our `pdb[p]` ext type/lowlevel-APIs and `mk_pdb()` factory.
- ._sigint: just our REPL-interaction shield-handler.
- ._tty_lock: containing all the root-actor TTY mutex machinery
including the `Lock`/`DebugStatus` primitives/APIs as well as the
inter-tree IPC context eps:
* the server-side `lock_stdio_for_peer()` which pairs with the,
* client-(subactor)-side `request_root_stdio_lock()` via the,
* pld-msg-spec of `LockStatus/LockRelease`.
AND the `any_connected_locker_child()` predicate.
Factoring the (basically duplicate) content from both use spots into
a common `@cm` which delivers a `bool` signalling whether the REPL
should be engaged. Fixes a lingering bug with `nullcontext()` calling
btw..
By supporting a new optional param to `open_crash_handler()`,
`raise_on_exit: bool|Sequence[Type[BaseException]] = True` which
determines whether, after the REPL interaction completes, the handled
exception is raised upward. This is **very** handy for writing bits of
"debug-able but resilient code" as is the case in (many) dependent
projects/apps.
Impl,
- `raise_on_exit` can be a `bool` or (set) sequence of types which will
always be raised.
- also add a `BoxedMaybeException.raise_on_exit` equiv which (for now)
we check matches (in case down the road we want to offer dynamic ctls).
- rename both crash-handler cm's `tb_hide` -> `hide_tb`.
It turns out to be fairly useful to allow hooking into a given actor's
entry-and-exit around `.devx._debug._pause/._post_mortem()` calls which
engage the `pdbp.Pdb` REPL (really our `._debug.PdbREPL` but yeah).
Some very handy use cases include,
- swapping out-of-band (config) state that may otherwise halt the
user's app since the actor normally handles kb&mouse input, in thread,
which means that the handler will be blocked while the REPL is in use.
- (remotely) reporting actor-runtime state for monitoring purposes
around crashes or pauses in normal operation.
- allowing for crash-handling to be hard-disabled via
`._state._runtime_vars` say for when you never want a debugger to be
entered in a production instance where you're not-sure-if/don't-want
per-actor `debug_mode: bool` settings to always be unset, say bc
you're still debugging some edge cases that ow you'd normally want to
REPL up.
Impl details,
- add a new optional `._state._runtime_vars['repl_fixture']` field which
for now can be manually set; i saw no reason for a formal API yet
since we want to convert the `dict` to a struct anyway (first).
- augment both `.devx._debug._pause()/._post_mortem()` with a new
optional `repl_fixture: AbstractContextManager[bool]` kwarg which
when provided is `with repl_fixture()` opened around the lowlevel
REPL interaction calls; if the enter-result, an expected `bool`, is
`False` then the interaction is hard-bypassed.
* for the `._pause()` case the `@cm` is opened around the entire body
of the embedded `_enter_repl_sync()` closure (for now) though
ideally longer term this entire routine is factored to be a lot less
"nested" Bp
* in `_post_mortem()` the entire previous body is wrapped similarly
and also now excepts an optional `boxed_maybe_exc: BoxedMaybeException`
only passed in the `open_crash_handler()` caller case.
- when the new runtime-var is overridden, (only manually atm) it is used
instead but only whenever the above `repl_fixture` kwarg is left null.
- add a `BoxedMaybeException.pformat() = __repr__()` which when
a `.value: Exception` is set renders a more "objecty" repr of the exc.
Obviously tests for all this should be coming soon!
Discovered this bug while testing `modden`'s daemon under various
cancelled-while-booting race conditions where sequential tests would
fail a lingering `assert 0` inside `.to_asyncio.run_as_asyncio_guest()`
to (oddly) catch redundant greenback-re-inits..
XD
Needs a test likely ;P
Such that the default is `None` and in the case where the caller *does
not* set the `pdb` arg to an explicit `bool` we instead determine it via
the output from `._state.is_debug_mode()` allowing for more "nonchalant"
usage throughout a (test) code base which passes the `debug_mode: bool`
as runtime config; allows delegation to the per-actor proc-global state.
Adding an underlying `iter_struct_ppfmt_lines()` (which can also be
called directly) to iter-render field-lines-pairs; it's now called from
the top level `.pformat()`. The use case is to allow more granular
control for rendering runtime primitive (like `Actor.pformat()`) reprs
depending on log-level/config, oh and using `textwrap` for indenting.
Particularly on a get-attr of `StackLevelAdapter.handlers` which, when
a `logger: StackLevelAdapter` is passed, we need to *not call* our own
`get_logger()` and just set is as the `log`. Fix the typing to match.
Since I'd like to use some `reprlib` formatting which `modden` already
implemented (and it's a main dependee project), figured I'd just bring
it all into `.devx.pformat` for now.
Moving it from where i (oddly) first wrote it up in `._entry` to a more
proper place with its pals in `.devx.pformat` ;p
Iface summary, what caller provides:
- `input_op`: a "sclang" chars-symbol to represent the conc "operation",
- `text`: the "entity" being *operated on*,
- `nest_prefix/indent`: what the ^ will be prefixed with.
- `prefix_op: bool` so that, when unset, the `input_op` is instead
used as a suffix to the first line of `text`.
- `next_indent: int|None = None` such that when set (and not null) we
use that exact ws-indent instead of calculating it from the
`len(nest_prefix)` allowing for specifying a `0`-indent easily.
- includes logic where we either get a non-zero value and
apply it strictly to both the `nest_prefix` and `text`, OR we
auto-calc it from any `nest_prefix`, NOT a conflation of both..
- `op_suffix: str = '\n'` for instead of assuming `f'{input_op}\n'`.
- `rm_from_first_ln: str` which allows removing chars from the
first line of `text` after a `str.strip()` (handy for removing
the '<Channel' first chevron from type-reprs).
There's also a huge comment-doc for "sclang" into the fn body which is
the terrible "primer" on this whole idea for the moment XD
We already have the `.ipc` sub-pkg name so it seems a bit
redundant/noisy for a namespace path Bp
Leave an alias for the `Server` rn since it's already used in a few
other internal mods.. will likely rename later if everyone is cool with
it..
Namely any CLI driven runtime-config fixtures such as,
- `--spawn-backend` and `start_method`,
- `--tpdb` and `debug_mode`,
- `--tpt-proto` and `tpt_protos`/`tpt_proto`,
- `reg_addr` as driven by the above.
This moves all fixtures and necessary hook funcs (CLI parsing,
configuring and test-gen) to the `._testing.pytest` module and thus
allows any dependent project to leverage these fixtures in their own
test suites after pointing to that plugin mod using,
```python
# conftest.py
pytest_plugins: tuple[str] = (
"tractor._testing.pytest",
)
```
Also, add a new `._testing.addr` helper mod which now contains
a factored `get_rando_addr()` helper for creating test-sesh unique
tpt-specific registry (or other) IPC endpoint addrs.
Oddly my env was borked bc a missing sub-dep (`typing-extensions`
apparently not added by `uv` for `stackscope`?) and then `stackscope`
was silently failing import and caused the shield-pause test to also
fail (since it couldn't match the expected `log.devx()` on console). The
import failure is not very explanatory due to the `log.warning()`;
change it to `.error()` level.
Also, explicitly import `_sync_pause_from_builtin` in
`examples/debugging/restore_builtin_breakpoint.py` to ensure the ref is
exported properly from `.devx.debug` (which it wasn't during dev of the
prior commit Bp).
Actually applying the input it in the root as well as all sub-actors by
passing it down to sub-actors through runtime-vars as delivered by the
initial `SpawnSpec` msg during child runtime init.
Impl deats,
- add a new `_state._runtime_vars['_enable_tpts']: list[str]` field set
by the input param (if provided) to `.open_root_actor()`.
- mk `current_ipc_protos()` return the runtime-var entry with instead
the default in the `_runtime_vars: dict` set to `[_def_tpt_proto]`.
- in `.open_root_actor()`, still error on this being a >1 `list[str]`
until we have more testing infra/suites to audit multi-protos per
actor.
- return the new value (as 3rd element) from `Actor._from_parent()` as
per the todo note; means `_runtime.async_main()` will allocate
`accept_addrs` as tpt-specific `Address` entries and pass them to
`IPCServer.listen_on()`.
Also,
- also add a new `_state._runtime_vars['_root_addrs']: list = []` field
with the intent of fully replacing the `'_root_mailbox'` field since,
* it will need to be a collection to support multi-tpt,
* it's a more cohesive field name alongside `_registry_addrs`,
* the root actor of every tree needs to have a dedicated addr set
(separate from any host-singleton registry actor) so that all its
subs can contact it for capabilities mgmt including debugger
access/locking.
- in the root, populate the field in `._runtime.async_main()` and for
now just set '_root_mailbox' to the first entry in that list in
anticipation of future multi-homing/transport support.
As per the outstanding TODO just above the redic `setattr()` loop in
`Actor._from_parent()`!!
Instead of all that risk-ay monkeying, add detailed comment-sections
around each explicit assignment of each `SpawnSpec` field, including
those that were already being explicitly set.
Those and other deats,
- ONLY enable the `.devx._debug` (CHERRY-CONFLICT: later changed to
`.debug._tty_lock`) module from `Actor.__init__()` in the root actor.
- ONLY enable the `.devx.debug._tty_lock` module from `Actor.__init__()`
in the root actor.
- add a new `get_mod_nsps2fps()` to replace the loop in init and assign
the initial `.enable_modules: dict[str, str]` from it.
- do `self.enable_modules.update(spawnspec.enable_modules)` instead of
an overwrite and assert the table is by default empty in all
subs.
As much as is possible given we currently do some graceful
cancellation join-waiting on any connected sub-actors whenever an active
`local_nursery: AcrtorNursery` in the post-rpc teardown sequence of
`handle_stream_from_peer()` is detected. In such cases we try to allow
the higher level inter-actor (task) context(s) to fully cancelled-ack
before conducting IPC machinery shutdown.
The main immediate motivation for all this is to support unit testing
the `.ipc._server` APIs but in the future may be useful for anyone
wanting to use our modular IPC transport layer sin-"actors".
Impl deats,
- drop passing an `actor: Actor` ref from as many routines in
`.ipc._server` as possible instead opting to use
`._state.current_actor()` where abs needed; thus the fns dropping an
`actor` input param are:
- `open_ipc_server()`
- `IPCServer.listen_on()`
- `._serve_ipc_eps()`
- `.handle_stream_from_peer()`
- factor the above mentioned graceful remote-cancel-ack waiting into
a new `maybe_wait_on_canced_subs()` which is called from
`handle_stream_from_peer()` and delivers a
maybe-`local_nursery: ActorNursery` for downstream logic; it's this
new fn which primarily still needs to call `current_actor()`.
- in `handle_stream_from_peer()` also use `current_actor()` to check if
a handshake is needed (or if it was called as part of some
actor-runtime-less operation like our unit test suite!).
- also don't pass an `actor` to `._rpc.process_messages()` see how-n-why
below..
Surrounding ipc-server client/caller adjustments,
- `._rpc.process_messages()` no longer takes an `actor` input and
now calls `current_actor()` instead.
- `._portal.open_portal()` is adjusted to ^.
- `._runtime.async_main()` is adjusted to the `.ipc._server`'s removal
of `actor` ref passing.
Also,
- drop some server `log.info()`s to `.runtime()`
Namely transferring the `Actor` peer-`Channel` tracking attrs,
- `._peers` which maps the uids to client channels (with duplicates
apparently..)
- the `._peer_connected: dict[tuple[str, str], trio.Event]` child-peer
syncing table mostly used by parent actors to wait on sub's to connect
back during spawn.
- the `._no_more_peers = trio.Event()` level triggered state signal.
Further we move over with some minor reworks,
- `.wait_for_peer()` verbatim (adjusting all dependants).
- factor the no-more-peers shielded wait branch-block out of
the end of `async_main()` into 2 new server meths,
* `.has_peers()` with optional chan-connected checking flag.
* `.wait_for_no_more_peers()` which *just* does the
maybe-shielded `._no_more_peers.wait()`
Call it `handle_stream_from_peer()` and bind in the `actor: Actor` via
a `handler=partial()` to `trio.serve_listeners()`.
With this (minus the `Actor._peers/._peer_connected/._no_more_peers`
attrs ofc) we get nearly full separation of IPC-connection-processing
(concerns) from `Actor` state. Thus it's a first look at modularizing
the low-level runtime into isolated subsystems which will hopefully
improve the entire code base's grok-ability and ease any new feature
design discussions especially pertaining to introducing and/or
composing-together any new transport protocols.
Just like we *were* for the `trio`-resource-errors it normally wraps
since we now also do the same wrapping in `MsgpackTransport.send()`
and we don't normally care to raise tpt-closure-errors on graceful actor
cancel requests.
Also, warn-report any non-tpt-closed low-level `trio` errors we haven't
yet re-wrapped (likely bc they haven't shown up).
Such that re-wrapping/raising from a low-level `trio` resource error is
simpler and includes the `.src_exc` in the `__repr__()` and
`.message/.args` rendered at higher layers (like from `Channel` and
`._rpc` machinery).
Impl deats,
- mainly leverages packing in a new cls-method `.repr_src_exc() -> str:`
repr of the underlying error before an optional `body: str` all as
handled by the previously augmented `.pformat()`'s delegation to
`pformat_exc()`.
- change `.src_exc` to be a property around a renamed `._src_exc`.
But wait, why?
- use it inside `MsgpackTransport.send()` to rewrap any
`trio.BrokenResourceError`s so we always see the underlying
`trio`-src-exc just like in the `.recv()._iter_packets()` handlers.
Primarily moving the `Actor._serve_forever()`-task-as-method and
supporting actor-instance attributes to a new `.ipo._server` sub-mod
which now encapsulates,
- the coupling various `trio.Nursery`s (and their independent lifetime mgmt)
to different `trio.serve_listener()`s tasks and `SocketStream`
handler scopes.
- `Address` and `SocketListener` mgmt and tracking through the idea of
an "IPC endpoint": each "bound-and-active instance" of a served-listener
for some (varied transport protocol's socket) address.
- start and shutdown of the entire server's lifetime via an `@acm`.
- delegation of starting/stopping tpt-protocol-specific `trio.abc.Listener`s
to the corresponding `.ipc._<proto_key>` sub-module (newly defined
mod-top-level instead of `Address` method) `start/close_listener()`
funcs.
Impl details of the `.ipc._server` sub-sys,
- add new `IPCServer`, allocated with `open_ipc_server()`, and which
encapsulates starting multiple-transport-proto-`trio.abc.Listener`s
from an input set of `._addr.Address`s using,
|_`IPCServer.listen_on()` which internally spawns tasks that delegate to a new
`_serve_ipc_eps()`, a rework of what was (effectively)
`Actor._serve_forever()` and which now,
* allocates a new `IPCEndpoint`-struct (see below) for each
address-listener pair alongside the specified
listener-serving/stream-handling `trio.Nursery`s provided by the
caller.
* starts and stops each transport (socket's) listener by calling
`IPCEndpoint.start/close_listener()` which in turn delegates to
the underlying `inspect.getmodule(IPCEndpoint.addr)` backend tpt
module's equivalent impl.
* tracks all created endpoints in a `._endpoints: list[IPCEndpoint]`
which is further exposed through public properties for
introspection of served transport-protocols and their addresses.
|_`IPCServer._[parent/stream_handler]_tn: Nursery`s which are either
allocated (in which case, as the same instance) or provided by the
caller of `open_ipc_server()` such that the same nursery-cancel-scope
controls offered by `trio.serve_listeners(handler_nursery=)` are
offered where the `._parent_tn` is used to spawn `_serve_ipc_eps()`
tasks, and `._stream_handler_tn` is passed verbatim as `handler_nursery`.
- a new `IPCEndpoint`-struct (as mentioned) which wraps each
transport-proto's address + listener + allocated-supervising-nursery
to encapsulate the "lifetime of a server IPC endpoint" such that
eventually we can track and managed per-protocol/address/`.listen_on()`-call
scoped starts/stops/restarts for the purposes of filtering/banning
peer traffic.
|_ also included is an unused `.peer_tpts` table which we can
hopefully use to replace `Actor._peers` in a `Channel`-tracking
transport-proto-aware way!
Surrounding changes to `.ipc.*` primitives to match,
- make `[TCP|UDS]Address` types `msgspec.Struct(frozen=True)` and thus
drop any-and-all `addr._host =` style mutation throughout.
|_ as such also drop their `.__init__()` and `.__eq__()` meths.
|_ UDS tweaks to field names and thus `.__repr__()`.
- move `[TCP|UDS]Address.[start/close]_listener()` meths to be mod-level
equiv `start|close_listener()` funcs.
- just hard code the `.ipc._types._key_to_transport/._addr_to_transport`
table entries instead of all the prior fancy dynamic class property
reading stuff (remember, "explicit is better then implicit").
Modified in `._runtime.Actor` internals,
- drop the `._serve_forever()` and `.cancel_server()`, methods and
`._server_down` waiting logic from `.cancel_soon()`
- add `.[_]ipc_server` which is opened just after the `._service_n` and
delegate to it for any equivalent publicly exposed instance
attributes/properties.
That is moving from `._addr`,
- `TCPAddress` to `.ipc._tcp`
- `UDSAddress` to `.ipc._uds`
Obviously this requires adjusting a buncha stuff in `._addr` to avoid
import cycles (the original reason the module was not also included in
the new `.ipc` subpkg) including,
- avoiding "unnecessary" imports of `[Unwrapped]Address` in various modules.
* since `Address` is a protocol and the main point is that it **does
not need to be inherited** per
(https://typing.python.org/en/latest/spec/protocol.html#terminology)
thus I removed the need for it in both transport submods.
* and `UnwrappedAddress` is a type alias for tuples.. so we don't
really always need to be importing it since it also kinda obfuscates
what the underlying pairs are.
- not exporting everything in submods at the `.ipc` top level and
importing from specific submods by default.
- only importing various types under a `if typing.TYPE_CHECKING:` guard
as needed.
By borrowing from the implementation of `RemoteActorError.pformat()`
which is now factored into a new `.devx.pformat_exc()` and re-used for
both error types while maintaining the same func-sig. Obviously delegate
`RemoteActorError.pformat()` to the new helper accordingly and keeping
the prior `body` generation from `.devx.pformat_boxed_tb()` as before.
The new helper allows for,
- passing any of a `header|message|body: str` which are all combined in
that order in the final output.
- getting the `exc.message` as the default `message` part.
- generating an objecty-looking "type-name" header to be rendered by
default when `header` is not overridden.
- "first-line-of `message`" processing which we split-off and then
re-inject as a `f'<{type(exc).__name__}( {first} )>'` top line header.
- an optional `tail: str = '>'` to "close the object"-look only added
when `with_type_header: bool = True`.
Adjustments to `TransportClosed` around this include,
- replacing the init `cause` arg for a `src_exc` which is now always
assigned to a same named instance var.
- displaying that new `.src_exc` in the `body: str` arg to the
`.devx.pformat.pformat_exc()` call so you can always see the
underlying (normally `trio`) source error.
- just make it inherit from `Exception` not `trio.BrokenResourceError`
to avoid handlers catching `TransportClosed` as the former
particularly in testing when we want to sometimes to distinguish them.
Since in hindsight the real analog of a net-proto's "bindspace"
(normally its routing layer's addresses-port-set) is more akin to the
"location in the file-system" for a UDS socket file (aka the file's
parent directory) determines whether or not the "port" (aka it's
file-name) collides with any other.
So the `._filedir: Path` is like the allocated "address" and,
the `._filename: Path|str` is basically the "port",
at least in my mind.. Bp
Thinking about fs dirs like a "host address" means you can get
essentially the same benefits/behaviour of say an (ip)
addresses-port-space but using the (current process-namespace's)
filesys-tree. Note that for UDS sockets in particular the
network-namespace is what would normally isolate so called "abstract
sockets" (i.e. UDS sockets that do NOT use file-paths by setting `struct
sockaddr_un.sun_path = 'abstract', see `man unix`); using directories is
even easier and definitely more explicit/readable/immediately-obvious as
a human-user.
As such this reworks all the necessary `UDSAddress` meths,
- `.unwrap()` now returns a `tuple(str(._filedir, str(._filename))`,
- `wrap_address()` now matches UDS on a 2nd tuple `str()` element,
- `.get_root()` no longer passes `maybe_pid`.
AND adjusts `MsgpackUDSStream` to,
- use the new `unwrap_sockpath()` on the `socket.get[sock/peer]name()`
output before passing directly as `UDSAddress.__init__(filedir, filename)`
instead of via `.from_addr()`.
- also pass `maybe_pid`s to init since no longer included in the
unwrapped-type form.
Such that the "global-ish" setting (actor-local) is managed with the
others per actor-process and type it as a `Literal['tcp', 'uds']` of the
currently support protocol keys.
Here obvi `_tpt` is some kinda shorthand for "transport" and `_proto` is
for "protocol" Bp
Change imports and refs in all dependent modules.
Oh right, and disable UDS in `wrap_address()` for the moment while
i figure out how to avoid the unwrapped type collision..
There was a very strange legacy test
`test_spawning.test_local_arbiter_subactor_global_state` which was
causing unforseen hangs/errors on the UDS tpt and looking deeper this
test was already doing root-actor things that should never have been
valid XD
So rework that test to properly demonstrate something of value
(i guess..) and add a new suite which start more rigorously auditing our
`open_root_actor()` permitted usage.
For the old test,
- since the main point of this test seemed to be the ability to invoke
the same function in both the parent and child actor (using the very
legacy `ActorNursery.run_in_actor()`.. due to be deprecated) rename it
to `test_run_in_actor_same_func_in_child`,
- don't re-enter `.open_root_actor()` since that's invalid usage (tested
in new suite see below),
- adjust some `spawn()` arg/var naming and ensure we only return in the
child.
For the new suite add tests for,
- ensuring the implicit `open_root_actor()` call under `open_nursery()`.
- double open of `open_root_actor()` from within the same process tree
both from a root and sub.
Intro some new `_exceptions` used in the new suite,
- a top level `RuntimeFailure` for generically expressing faults not of
our own doing that prevent successful operation; this is what we now
(changed in this commit) raise on attempts to open a 2nd root.
- mk `ActorFailure` derive from the former; it's already used from
`._spawn` when subprocs fail to boot.
- aggregate the `MsgStream.aclose()` "reader tasks" stats content into a
common `message: str` before emit.
- tweak an `_rpc.process_messages()` emit per new `Channel.__repr__()`.
Throughout `_context` and `_spawn` where it causes no big disruption.
Still lots to work out for things like how to pass `--uid
<tuple-as-str>` to spawned subactors and whether we want a diff name for
the minimum `tuple` required to distinguish a subactor pre-process-ID
allocation by the OS.
Finally.. i've been meaning todo this for ages since the
actor-id-swap-as-handshake is better layered as part of the IPC msg-ing
machinery and then let's us encapsulate the connection-time-assignment
of a remote peer's `Aid` as a new `Channel.aid: Aid`. For now we
continue to offer the `.uid: tuple[str, str]` attr (by delegating to the
`.uid` field) since there's still a few things relying on it in the
runtime and ctx layers
Nice bonuses from this,
- it's very easy to get the peer's `Aid.pid: int` from anywhere in an
IPC ctx by just reading it from the chan.
- we aren't saving more then the wire struct-msg received.
Also add deprecation warnings around usage to get us moving on porting
the rest of consuming runtime code to the new attr!
For the case where there's clearly no socket file created/bound
obviously the `trio.socket.connect()` call will raise
`FileNotFoundError`, so just translate this to
a builtin-`ConnectionError` at the transport layer so we can report the
guilty `UDSAddress`.
The more I think about it, it seems @guille's orig approach of
unwrapping UDS socket-file addresses to strings (or `Path`) is making
the most sense. I had originally thought that pairing it with the
listening side's pid would add clarity (and it definitely does for
introspection/debug/logging) but since we don't end up passing that pid
to the eventual `.connect()` call on the client side, it doesn't make
much sense to wrap it for the wire just to discard.. Further, the
`tuple[str, int]` makes `wrap_address()` break for TCP since it will
always match on uds first.
So, on that note this patch refines a few things in prep for going back
to that original `UnwrappedAddress` as `str` type though longer run
i think the more "builtin approach" would be to add `msgspec` codec
hooks for these types to avoid all the `.wrap()`/`.unwrap()` calls
throughout the runtime.
Down-low deats,
- add `wrap_address()` doc string, detailed (todo) comments and handle
the `[None, None]` case that can come directly from
`._state._runtime_vars['_root_mailbox']`.
- buncha adjustments to `UDSAddress`,
- add a `filedir`, chng `filepath` -> `filename` and mk `maybe_pid` optional.
- the intent `filedir` is act as the equivalent of the host part in a network proto's
socket address and when it's null use the `.def_bindspace = get_rt_dir()`.
- always ensure the `filedir / filename` is an absolute path and
expose it as a new `.sockpath: Path` property.
- mk `.is_valid` actually verify the `.sockpath` is in the valid
`.bindspace: namely just checking it's in the expected dir.
- add pedantic `match:`ing to `.from_addr()` such that we error on
unexpected `type(addr)` inputs and otherwise parse any `sockpath:
Path` inputs using a new `unwrap_sockpath()` which simply splits an
abs file path to dir, file-name parts.
- `.unwrap()` now just `str`-ifies the `.sockpath: Path`
- adjust `.open/close_listener()` to use `.sockpath`.
Call it `maybe_block_bp()` can wrap the `open_root_actor()` body with
it. Main reason is to guarantee we can bp inside actor runtime bootup as
needed when debugging internals! Prolly should factor this to another
module tho?
ALSO, ensure we RTE on recurrent entries to `open_root_actor()` from
within an existing tree! There was actually `test_spawning` test somehow
getting away with this!? Should never be possible or allowed!
And map `.__repr__/__str__` to it and add various new fields to fill it
out,
- drop `self.uid` as var and instead add `Actor._aid: Aid` and proxy to
it for the various `.name/.uid/.pid` properties as well as a new
`.aid` field.
|_ the `Aid.pid` addition is also included.
Other improvements,
- flip to a sync call to `Address.close_listener()`.
- track the `async_main()` parent task as `Actor._task`.
- add exception logging around failure to bind due to already-in-use
when calling `add.open_listener()` in `._stream_forever()`; sometimes
the error might be overridden by something else during the
runtime-failure unwind..
Namely reducing the duplication of class-fields and `TypeVar`s used
for parametrizing the `Address` protocol type,
- drop all of the `TypeVar` types and just stick with all concrete addrs
types inheriting from `Address` only.
- rename `Address.name_key` -> `.proto_key`.
- rename `Address.address_type` -> `.unwrapped_type`
- rename `.namespace` -> `.bindspace` to better reflect that this "part"
of the address represents the possible "space for binding endpoints".
|_ also linux already uses "namespace" to mean the `netns` and i'd
prefer to stick with their semantics for that.
- add `TCPAddress/UDSAddress.def_bindspace` values.
- drop commented `.open_stream()` method; never used.
- simplify `UnwrappedAdress` to just a `tuple` of union types.
- add logging to `USDAddress.open_listener()` for now.
- adjust `tractor.ipc/_uds/tcp` transport to use new addr field names.
Much like we already do in the `._iter_packets()` async-generator which
delivers to `.recv()` and `async for`, handle the `''[Errno 32] Broken
pipe'` case that can show up with unix-domain-socket usage.
Seems like the cause is due to how fast the socket can be torn down
during a registry addr channel ping where,
- the sending side can break the connection faster then the pong side
can prep its handshake msg,
- the pong side tries to send it's handshake pkt via
`.SocketStream.send_all()` after the breakage and then raises
`trio.BrokenResourceError`.
Such that whenev the `self._ctx.chan._exc is trans_err` we suppress.
I.e. when the `Channel._exc: Exception|None` error **is the same as**
set by the `._rpc.process_messages()` loop (that is, set to the
underlying transport layer error), we suppress the lowlevel tb,
otherwise we deliver the full tb since likely something at the lowlevel
that we aren't detecting changed/signalled/is-relevant!
Much like how `Context` has been implemented, try to give tons of high
level details on all the lower level encapsulated primitives, namely the
`.msgstream/.transport` and any useful runtime state.
B)
Impl deats,
- adjust `.from_addr()` to only call `._addr.wrap_address()` when we
detect `addr` is unwrapped.
- add another `log.runtime()` using the new `.__repr__()` in
`Channel.from_addr()`.
- change to `UnwrappedAddress` as in prior commits.
Previously whenever an `ActorNursery.start_actor()` call did not receive
a `bind_addrs` arg we would allocate the default `(localhost, 0)` pairs
in the parent, for UDS this obviously won't work nor is it ideal bc it's
nicer to have the actor to be a socket server (who calls
`Address.open_listener()`) define the socket-file-name containing their
unique ID info such as pid, actor-uuid etc.
As such this moves "random" generation of server addresses to the
child-side of a subactor's spawn-sequence when it's sin-`bind_addrs`;
i.e. we do the allocation of the `Address.get_random()` addrs inside
`._runtime.async_main()` instead of `Portal.start_actor()` and **only
when** `accept_addrs`/`bind_addrs` was **not provided by the spawning
parent**.
Further this patch get's way more rigorous about the `SpawnSpec`
processing in the child inside `Actor._from_parent()` such that we
handle any invalid msgs **very loudly and pedantically!**
Impl deats,
- do the "random addr generation" in an explicit `for` loop (instead of
prior comprehension) to allow for more detailed typing of the layered
calls to the new `._addr` mod.
- use a `match:/case:` for process any invalid `SpawnSpec` payload case
where we can instead receive a `MsgTypeError` from the `chan.recv()`
call in `Actor._from_parent()` to raise it immediately instead of
triggering downstream type-errors XD
|_ as per the big `#TODO` we prolly want to take from other callers
of `Channel.recv()` (like in the `._rpc.process_messages()` loop).
|_ always raise `InternalError` on non-match/fall-through case!
|_ add a note about not being able to use `breakpoint()` in this
section due to causality of `SpawnSpec._runtime_vars` not having
been processed yet..
|_ always return a third element from `._from_rent()` eventually to be
the `preferred_transports: list[str]` from the spawning rent.
- use new `._addr.mk_uuid()` and pass to new `Actor.__init__(uuid: str)`
for all actor creation (including in all the mods tweaked here).
- Move to new type-alias-name `UnwrappedAddress` throughout.
Such that any UDS socket pair is represented (and with the recent
updates to) a `USDAddress` via a similar pair-`tuple[str, int]` as TCP
sockets, a pair of the `.filepath: Path` & the peer proc's `.pid: int`
which we read from the underlying `socket.socket` using
`.set/getsockopt()` calls
Impl deats,
- using the Linux specific APIs, we add a `get_peer_info()` which reads
the `(pid, uid, gid)` using the `SOL_SOCKET` and `SOL_PEECRED` opts to
`sock.getsockopt()`.
|_ this presumes the client has been correspondingly configured to
deliver the creds via a `sock.setsockopt(SOL_SOCKET, SO_PASSCRED,
1)` call - this required us to override `trio.open_unix_socket()`.
- override `trio.open_unix_socket()` as per the above bullet to ensure
connecting peers always transmit "credentials" options info to the
listener.
- update `.get_stream_addrs()` to always call `get_peer_info()` and
extract the peer's pid for the `raddr` and use `os.getpid()` for
`laddr` (obvi).
|_ as part of the new impl also `log.info()` the creds-info deats and
socket-file path.
|_ handle the oddity where it depends which of `.getpeername()` or
`.getsockname()` will return the file-path; i think it's to do with
who is client vs. server?
Related refinements,
- set `.layer_key: int = 4` for the "transport layer" ;)
- tweak some typing and multi-line unpacking in `.ipc/_tcp`.
A few things that can fundamentally change,
- UDS addresses now always encapsulate the local and remote pid such
that it denotes each side's process much like a TCP *port*.
|_ `.__init__()` takes a new `maybe_pid: int`.
|_ this required changes to the `.ipc._uds` backend which will come in
an subsequent commit!
|_ `UDSAddress.address_type` becomes a `tuple[str, int]` just like the
TCP case.
|_ adjust `wrap_address()` to match.
- use a new `_state.get_rt_dir() -> Path` as the default location for
UDS socket file: now under `XDG_RUNTIME_DIR'/tractor/` subdir by
default.
- re-implement `USDAddress.get_random()` to use both the local
`Actor.uid` (if available) and at least the pid for its socket file
name.
Removals,
- drop the loop generated `_default_addrs`, simplify to just
`_default_lo_addrs` for per-transport default registry addresses.
|_ change to `_address_types: dict[str, Type[Address]]` instead of
separate types `list`.
|_ adjust `is_wrapped_addr()` to just check `in _addr_types.values()`.
- comment out `Address.open_stream()` it's unused and i think the wrong
place for this API.
Renames,
- from `AddressTypes` -> `UnwrappedAddress`, since it's a simple type
union and all this type set is, is the simple python data-structures
we encode to for the wire.
|_ see note about possibly implementing the `.[un]wrap()` stuff as
`msgspec` codec `enc/dec_hook()`s instead!
Additions,
- add a `mk_uuid()` to be used throughout the runtime including for
generating the `Aid.uuid` part.
- tons of notes around follow up refinements!
EventFD class now expects the fd to already be init with open_eventfd
RingBuff Sender and Receiver fully manage SharedMemory and EventFD lifecycles, no aditional ctx mngrs needed
Separate ring buf tests into its own test bed
Add parametrization to test and cancellation
Add docstrings
Add simple testing data gen module .samples
Synchronizing the "main", previously (and less woke-ly/succinctly)
called "master", branch between our `gitea` remote and the current
`github` tip.
* main:
Only set shield flag when trio nursery mode is used
Disable parent channel append on get_peer_by_name to_scan
First attempt at getting `multiprocessing.shared_memory.ShareableList`
working; we wrap the stdlib type with a readonly attr and a `.key` for
cross-actor lookup. Also, rename all `numpy` specific routines to have
a `ndarray` suffix in the func names.
More or less a verbatim copy-paste minus some edgy variable naming and
internal `piker` module imports. There is a bunch of OHLC related
defaults that need to be dropped and we need to adjust to an optional
dependence on `numpy` by supporting shared lists as per the mp docs.
In particular ensuring we use `ctx._pld_rx.recv_msg_nowait()` from
`.receive_nowait()` (which is called from `.aclose()`) such that we
ALWAYS (can) set the surrounding `Context._result/._outcome_msg` attrs
on reception of a final `Return`!!
This fixes a final stream-teardown-race-condition-bug where prior we
normally didn't set the `Context._result/._outcome_msg` in such cases.
This is **precisely because** `.receive_nowait()` only returns the
`pld` and when called from `.aclose()` this value is discarded, meaning
so is its boxing `Return` despite consuming it from the underlying
`._rx_chan`..
Longer term this should be solved differently by ensuring such races
cases are handled at a higher scope like inside `Context._deliver_msg()`
or the `Portal.open_context()` enter/exit blocks? Add a detailed warning
note and todos for all this around the special case block!
Such that any `Return` is always capture for each ctx instance and set
in `._deliver_msg()` normally; ensures we can at least introspect for it
when missing (like in a recently discovered stream teardown race bug).
Yes this augments the already existing `._result` which is dedicated for
the `._outcome_msg.pld` in the non-error case; we might want to see if
there's a nicer way to directly proxy ref to that without getting the
pre-pld-decoded `Raw` form with `msgspec`?
Also use the new `ctx._pld_rx.recv_msg()` and drop assigning
`pld_rx._ctx`.
Namely renaming and tweaking the `MsgType` receiving methods,
- `.recv_msg()` from what was `.recv_msg_w_pld()` which both receives
the IPC msg from the underlying `._rx_chan` and then decodes its
payload with `.decode_pld()`; it now also log reports on the different
"stage of SC dialog protocol" msg types via a `match/case`.
- a new `.recv_msg_nowait()` sync equivalent of ^ (*was*
`.recv_pld_nowait()`) who's use was the source of a recently
discovered bug where any final `Return.pld` is being
consumed-n-discarded by by `MsgStream.aclose()` depending on
ctx/stream teardown race conditions..
Also,
- remove all the "instance persistent" ipc-ctx attrs, specifically the
optional `_ipc`, `_ctx` and the `.wraps_ipc()` cm, since none of them
were ever really needed/used; all methods which require
a `Context/MsgStream` are explicitly always passed.
- update a buncha typing namely to use the more generic-styled
`PayloadT` over `Any` and obviously `MsgType[PayloadT]`.
Repairs a bug in `drain_to_final_msg()` where in the `Yield()` case
block we weren't guarding against the `ctx._stream is None` edge case
which should be treated a `continue`-draining (not a `break` or
attr-error!!) situation since the peer task maybe be continuing to send
`Yield` but has not yet sent an outcome msg (one of
`Return/Error/ContextCancelled`) to terminate the loop. Ensure we
explicitly warn about this case as well as `.cancel()` emit on a taskc.
Thanks again to @guille for discovering this!
Also add temporary `.info()`s around rxed `Return` msgs as part of
trying to debug a different bug discovered while updating the
context-semantics test suite (in a prior commit).
Just like we do from the `.devx._debug.open_crash_handler()`, this
allows checking various attrs on the raised `ContextCancelled` much like
`with pytest.raises() as excinfo:`.
Seems this can happen in particular when we raise a `MessageTypeError`
on the sender side of a `Context`, since there isn't any msg relayed
from the other side (though i'm wondering if MTE should derive from RAE
then considering this case?).
Means `RemoteActorError.boxed_type = None` in such cases instead of
raising an attr-error for the `None.boxed_type_str`.
Since it should only be used from within a `Portal.open_context()`
scope, make sure the caller knows that!
Also don't hide the frame in tb if the immediate function errors..
By using our new `PldRx` design we can,
- pass through the pld-spec & a `dec_hook()` to our `MsgDec` which is
used to configure the underlying `.dec: msgspec.msgpack.Decoder`
- pass through a `enc_hook()` to `mk_codec()` and use it to conf the
equiv `MsgCodec.enc` such that sent msg-plds are converted prior
to transport.
The trick ended up being just to always union the `mk_dec()`
extension-types spec with the normaly with the `msgspec.Raw` pld-spec
such that the `dec_hook()` is only invoked for payload types tagged
by the encoder/sender side B)
A variety of impl tweaks to make it all happen as well as various
cleanups in the `.msg._codec` mod include,
- `mk_dec()` no defaul `spec` arg, better doc string, accept the new
`ext_types` arg, doing the union of that with `msgspec.Raw`.
- proto-ed a now unused `mk_boxed_ext_struct()` which will likely get
removed since it ended up that our `PayloadMsg` structs already cover
the ext-type-hook requirement that the decoder is passed
a `.type=msgspec.Struct` of some sort in order for `.dec_hook` to be
used.
- add a `unpack_spec_types()` util fn for getting the `set[Type]` from
from a `Union[Type]` annotation instance.
- mk the default `mk_codec(pc_pld_spec = Raw,)` since the `PldRx` design
was already passing/overriding it and it doesn't make much sense to
use `Any` anymore for the same reason; it will cause various `Context`
apis to now break.
|_ also accept a `enc_hook()` and `ext_types` which are used to maybe
config the `.msgpack.Encoder`
- generally tweak a bunch of comments-as-docs and todos namely the ones
that are completed after the pld-rx design was implemented.
Also,
- mask the non-functioning `'defstruct'` approach `inside
`.msg.types.mk_msg_spec()` to prep for its removal.
Adjust the test suite (rn called `test_caps_based_msging`),
- add a new suite `test_custom_extension_types` and move and
use the `enc/dec_nsp()` hooks to the mod level for its use.
- prolly planning to drop the `test_limit_msgspec` suite since it's
mostly replaced by the `test_pldrx_limiting` mod's version?
- originally was tweaking a bunch in `test_codec_hooks_mod` but likely
it will get mostly rewritten to be simpler and simply verify that
ext-typed fields can be used over IPC `Context`s between actors (as
originally intended for this sub-suite).
Since it isn't required until the landing of the new service-manager
stuff in #12; was an oversight
from commit `0607a31dddeba032a2cf7d9fe605edd9d7bb4846`.
Like any `bdb.BdbQuit` that might be relayed from a remote context after
a REPl exit with the `quit` cmd. This fixes various issues while
debugging where it may not be clear to the parent task that the child
was terminated with a purposefully unrecoverable error.
Inside a new `.trionics._beg` and exposed from the subpkg ns in
anticipation of the `strict_exception_groups=False` being removed by
`trio` in py 3.15.
Notes,
- mk an embedded single-exc "extractor" using a `BaseExceptionGroup.exceptions` length
check, when 1 return the lone child.
- use the above in a new `@acm`, async bc it's most likely to be composed in an
`async with` tuple-style sequence block, called `collapse_eg()` which
acts a one line "absorber" for when the above mentioned flag is no
logner supported by `trio.open_nursery()`.
All untested atm fwiw.. but soon to be used in our test suite(s) likely!
Namely when the subactor fails to lock the root, in which case we
try to be very verbose about how/what failed in logging as well
as ensure we cancel the employed IPC ctx.
Implement the outer `BaseException` handler to handle both styles,
- match on an eg (or the prior std cancel excs) only raising a lone
sub-exc from for former.
- always `as _req_err:` and assign to a new func-global `req_err`
to enable the above matching.
Other,
- raise `DebugStateError` on `status.subactor_uid != actor_uid`.
- fix a `_repl_fail_report` ref error due to making silly assumptions
about the `_repl_fail_msg` global; now copy from global as default.
- various log-fmt and logic expression styling tweaks.
- ignore `trio.Cancelled` by default in `open_crash_handler()`.
Namely inside,
- `ActorNursery.open_portal()` which uses
`.trionics.maybe_open_nursery()` and is now adjusted to
pass-through `**kwargs` for at least this flag.
- inside the `.trionics.gather_contexts()`.
Such that it gets passed through to `.open_root_actor()` in the
`implicit_runtime==True` case - useful for debugging cases where
`.devx._debug` APIs might be used to avoid REPL clobbering in subactors.
Since it'll likely need a bit of detailing to get the test suite running
identically with strict egs (exception groups), i've opted to just flip
the switch on a few core nursery scopes for now until as such a time
i can focus enough to port the matching internals.. Xp
Such that a `TrioCancelled` is raised in the aio task via
`.set_exception()` to explicitly indicate and allow that task to handle
a taskc request from the parent `trio.Task`.
Seems that on 3.13 it's not showing our script code in the output now?
Gotta get an example for @oremanj to see what's up but really it'd be
nice to just custom format stuff above `trio`'s runtime by def..
Anyway, update the `.devx._stackscope`,
- log formatting to be a little more "sclangy" lookin.
- change the per-actor "delimiter" lines style.
- report the `signal.getsignal(SIGINT)` which i needed in the
`sync_bp.py` with ctl-c causing a hang..
- mask the `_tree_dumped` duplicator log report as well as the "dumped
fine" one.
- add an example `pkill --signal SIGUSR1` cmdline.
Tweak the test to cope with,
- not showing our script lines now.. which i've commented in the
`assert_before()` patts..
- to expect the newly formatted delimiter (ascii) lines to separate the
root vs. hanger sub-actor sections.
That is whenever `trio.EndOfChannel` is raised (presumably from the
`._to_trio.receive()` call inside `LinkedTaskChannel.receive()`) we need
to be extra certain that we let it bubble upward transparently DESPITE
special exc-as-signal handling that is normally suppressed from the aio
side; REPEAT we want to ALWAYS bubble any `trio_err ==
trio.EndOfChannel` in the `finally:` handler of `translate_aio_errors()`
despite `chan._trio_to_raise == AsyncioTaskExited` such that the
caller's iterable machinery will operate as normal when the inter-task
stream is stopped (again, presumably by the aio side task terminating
the inter-task stream).
Main impl deats for this,
- in the EoC handler block ensure we assign both `chan._trio_err` and
the local `trio_err` as well as continue to re-raise.
- add a case to the match block in the `finally:` handler which FOR SURE
re-raises any `type(trio_err) is EndOfChannel`!
Additionally fix a bad bug,
- a ref bug where we were NOT using the
`except BaseException as _trio_err` to assign to `chan._trio_err` (by
accident was missing the leading `_`..)
Unrelated impl tweak,
- move all `maybe_raise_aio_side_err()` content back to inline with its
parent func - makes it easier to use `tractor.pause()` mostly Bp
- go back to trying to use `aio_task.set_exception(aio_taskc)` for now
even though i'm pretty sure we're going to move to a try-fute-first
style helper for this in the future.
Adjust some tests to match/mk-them-green,
- break from `aio_echo_server()` recv loop on
`to_asyncio.TrioTaskExited` much like how you'd expect to (implicitly
with a `for`) with a `trio.EndOfChannel`.
- toss in a masked `value is None` pause point i needed for debugging
inf looping caused by not re-raising EoCs per the main patch
description.
- add a debug-mode sized delay to root-infected test.
Such that any combination of task terminations/exits can be explicitly
handled and "dual side independent" crash cases re-raised in egs.
The main error-or-exit impl changes include,
- use of new per-side "signaling exceptions":
- TrioTaskExited|TrioCancelled for signalling aio.
- AsyncioTaskExited|AsyncioCancelled for signalling trio.
- NOT overloading the `LinkedTaskChannel._trio/aio_err` fields for
err-as-signal relay and instead add a new pair of
`._trio/aio_to_raise` maybe-exc-attrs which allow each side's
task to specify what it would want the other side to raise to signal
its/a termination outcome:
- `._trio_to_raise: AsyncioTaskExited|AsyncioCancelled` to signal,
|_ the aio task having returned while the trio side was still reading
from the `asyncio.Queue` or is just not `.done()`.
|_ the aio task being self or trio-request cancelled where
a `asyncio.CancelledError` is raised and caught but NOT relayed
as is back to trio; instead signal a "more explicit" exc type.
- `._aio_to_raise: TrioTaskExited|TrioCancelled` to signal,
|_ the trio task having returned while the aio side was still reading
from the mem chan and indicating that the trio side might not
care any more about future streamed values (like the
`Stop/EndOfChannel` equivs for ipc `Context`s).
|_ when the trio task canceld we do
a `asyncio.Future.set_exception(TrioTaskExited())` to indicate
to the aio side verbosely that it should cancel due to the trio
parent.
- `_aio/trio_err` are now left to only capturing the **actual**
per-side task excs for introspection / other side's handling logic.
- supporting "graceful exits" depending on API in use from
`translate_aio_errors()` such that if either side exits but the other
side isn't expect to consume the final `return`ed value, we just exit
silently, which required:
- adding a `suppress_graceful_exits: bool` flag.
- adjusting the `maybe_raise_aio_side_err()` logic to use that flag
and suppress only on certain combos of `._trio_to_raise/._trio_err`.
- prefer to raise `._trio_to_raise` when the aio-side is the src and
vice versa.
- filling out pedantic logging for cancellation cases indicating which
side is the cause.
- add a `LinkedTaskChannel._aio_result` modelled after our
`Context._result` a a similar `.wait_for_result()` interface which
allows maybe accessing the aio task's final return value if desired
when using the `open_channel_from()` API.
- rename `cancel_trio()` done handler -> `signal_trio_when_done()`
Also some fairly major test suite updates,
- add a `delay: int` producing fixture which delivers a much larger
timeout whenever `debug_mode` is set so that the REPL can be used
without a surrounding cancel firing.
- add a new `test_aio_exits_early_relays_AsyncioTaskExited` including
a paired `exit_early: bool` flag to `push_from_aio_task()`.
- adjust `test_trio_closes_early_causes_aio_checkpoint_raise` to expect
a `to_asyncio.TrioTaskExited`.
The (rare) condition is heavily detailed in new comments in
the `cancel_trio()` callback but, more or less the idea here is to be
extra pedantic in raising an `Exceptiongroup` of errors from each task
(both `asyncio` and `trio`) whenever the 2 tasks raise "independently"
- in the sense that it's not obviously one side's task causing an error
(or cancellation) in the other. In this case we set the error for each
side on the `LinkedTaskChannel` (via new attrs described later).
As a synopsis, most of this work was refined out of supporting
`infected_aio=True` mode in the **root actor** and in particular as part
of getting that to work inside the `modden` daemon which at the time of
writing was still using the `i3ipc` lib and thus `asyncio`.
Impl deats,
- extend the `LinkedTaskChannel` field/API set (and type it),
- `._trio_task: trio.Task` for test/user introspection.
- also "stage" some ideas for a more refined interface,
- `.started()` to deliver the value yielded to the `trio.Task` parent.
|_ also includes some todos for how to implement this design
underneath.
- `._aio_first: Any|None = None` to hold that value ^.
- `.wait_aio_complete()` for syncing to the asyncio task.
- some detailed logging around "asyncio cancelled trio" case.
- Move `AsyncioCancelled` in this module.
Styling changes,
- generally more explicit var naming.
- some todos for getting modern and fancy with typing..
NB, Let it be known this commit msg was written on a friday with the
help of various "mr. white" solns.
Along the lines of something like `pytest.raises()` where the handled
exception can be inspected from the `pdbp` REPL using its `.value` field
B)
This is super handy in particular for understanding
`BaseException[Group]`s without manually adding surrounding handler code
to assign the `except[*] Exception as exc_var:` particularly when trying
to understand multi-cancelled eg trees.
Since we can't use it to `Task.set_exception()` (since that task method never
seems to work.. XD) and setting the private/internal always seems to do
the desired raising in the task? I realize it's an internal `asyncio`
runtime field but i'd rather take the risk of it breaking then having to
rely on our own equivalent hack..
Also, it seems like the case where the task's associated (and internal)
future-waiter field is null, we won't run into the (same?) prior hanging
issues (maybe since there's nothing for `asyncio` internals to use to
wait XD ??) when `Task.cancel()` is used..??
Main deats,
- add and `Future.set_exception()` a new signal-exception
`class TrioTaskExited(AsyncioCancelled):` whenever the trio-task exits
gracefully and the asyncio-side task is still doing blocking work (of
some sort) which *seem to* be predicated by a check that
`._fut_waiter is not None`.
- always call `asyncio.Queue.shutdown()` for the same^ as well as
whenever we decide to call `Task.cancel()`; in that case the shutdown
relays correctly?
Some further refinements,
- only warn about `Task.cancel()` usage when actually used ;)
- more local scope vars setting in the exit phase of
`translate_aio_errors()`.
- also in ^ use explicit caught-exc var names for each error-type.
Since it can not only cause the guest-mode run to abandon but also in
some edge cases prevent `trio`-errors from propagating (at least on
py3.12-13?) as discovered as part of supporting this mode officially
in the *root actor*.
As such try to avoid that method as much as possible instead opting to
pass the `trio`-side error via the iter-task channel ref.
Deats,
- add a `LinkedTaskChannel._trio_err: BaseException|None` which gets set
whenver the `trio.Task` error is caught; ONLY set `AsyncioCancelled`
when the `trio` task was for sure the cause, whether itself cancelled
or errored.
- always check for this error when exiting the `asyncio` side (even when
terminated via a call to `asyncio.Task.cancel()` or during any other
`CancelledError` handling such that the `asyncio`-task can expect to
handle `AsyncioCancelled` due to the above^^ cases.
- never `cs.cancel()` the `trio` side unless that cancel scope has not
yet been `.cancel_called` whatsoever; it's a noop anyway.
- only raise any exc from `asyncio.Task.result()` when `chan._aio_err`
does not already match it since the existence of the pre-existing
`task_err` means `asyncio` prolly intends (or has already) raised and
interrupted the task elsewhere.
Various supporting tweaks,
- don't bother maybe-init-ing `greenback` from the actor entrypoint
since we already need to (and do) bestow the portals to each `asyncio`
task spawned using the `run_task()`/`open_channel_from()` API; further
the init-ing should be done already by client code that enables
infected mode (even in the root actor).
|_we should prolly also codify it from any
`run_daemon(infected_aio=True, debug_mode=True)` usage we offer.
- pass all the `_<field>`s to `Linked TaskChannel` explicitly in named
kwarg style.
- better sclang-style log reports throughout, particularly on teardowns.
- generally more/better comments and docs around (not well understood)
edge cases.
- prep to just inline `maybe_raise_aio_side_err()` closure..
When `.pause_from_sync()` is called from an `asyncio.Task` which was
never bestowed a portal we want to be mega pedantic about it; indicate
that the task was NOT spawned from our `.to_asyncio` API and likely by
some out-of-our-control code (normally using
`asyncio.ensure_future()/.create_task()`). Though `greenback` already
errors on such usage, it's not always clear why no portal exists;
explaining the situation of a 3rd-party-bg-spawned-task should avoid
dev confusion for most cases.
Impl deats,
- distinguish between an actor in infected mode versus the actual caller
of `.pause_from_sync()` being an `asyncio.Task` with more explicit
`asyncio_task` and `is_infected_aio` vars.
- ONLY in the case of being both an infected-mode-actor AND detecting
that the caller is an `asyncio.Task`, check `greenback.has_portal()`
such that when not bestowed we presume the aforementioned
3rd-party-bg-task case above and raise a new explicit RTE with
a detailed explanatory message.
- add some masked draft code for handling the speical case of a root
actor `asyncio.Task` caller which could (in theory) not actually
require gb portal use since the `Lock` can be acquired directly
without IPC.
|_this will likely require factoring of various pause machinery funcs
into a `_pause_from_root_task()` to mk the impl sane XD
Other,
- expose a new `debug_filter: Callable` which can be provided by the
caller of `_maybe_enter_pm()` to predicate whether to enter the
debugger REPL based on the caught `BaseException|BaseExceptionGroup`;
this is handy for customizing the meaning of "graceful cancellations"
so as to avoid crash handling on expected egs of more then
`trioCancelled`.
|_ make the default as it was implemented: `not is_multi_cancelled(err)`
- pass-through a new `ignore: set[BaseException]` as
`open_crash_handler(ignore_nested=ignore)` to allow for the same
silent-cancellation-egs-swallowing as desired from outside the actor
runtime.
Such that equivalents of `trio.Cancelled` from other runtimes such as
`asyncio.CancelledError` and `subprocess.CalledProcessError` (with
a `.returncode == -2`) can be gracefully ignored as needed by the
caller.
For example this is handy if you want to avoid debug-mode REPL entry on
an exception-group full of only some subset of exception types since you
expect certain tasks to raise such errors after having been cancelled by
a request from some parent supervision sys (some "higher up"
`trio.CancelScope`, a remote triggered `ContextCancelled` or just from
and OS SIGINT).
Impl deats,
- offer a new `ignore_nested: set[BaseException]` param which by
default we add `trio.Cancelled` to when no other types are provided.
- use `ExceptionGroup.subgroup(tuple(ignore_nested)` to filter to egs of
the "ignored sub-errors set" and return any such match (instead of
`True`).
- detail a comment on exclusion case.
Such that we can hook into 3rd-party-libs more easily to monkey them and
use our (prettier/hipper) console logging with something like (an
example from the client project `modden`),
```python
connection_mod = i3ipc.connection
tractor_style_i3ipc_logger: logging.LoggingAdapter = tractor.log.get_console_log(
_root_name=connection_mod.__name__,
logger=i3ipc.connection_mod.logger,
level='info',
)
# monkey the instance-ref in 3rd-party module
connection_mod.logger = our_logger
```
Impl deats,
- expose as `get_console_log(logger: logging.Logger)` and add default
failover logic.
- toss in more typing, also for mod-global instance.
Such that you can use,
```python
tractor.to_asyncio.run_as_asyncio_guest(
trio_main=_trio_main,
)
```
to boostrap the root actor (and thus main parent process) to embed
the actor-rumtime into an `asyncio` loop. Prove it all works with an
subactor-free version of the aio echo-server test suite B)
For comparing a `msgspec.Struct` against an input `dict` presumably to
be used as input for struct instantiation. The main diff with
`.__sub__()` is that non-existing fields on either are reported
(loudly).
Such that maybe we can eventually offer a nicer higher-level API which
implements much of the boilerplate required by `msgspec` (like
type-matched branching to serialization logic) via a type-table
interface or something?
Not sure if the idea is that useful so leaving it all as TODOs for now
obviously.
Mostly fixing edge cases with `asyncio` and/or bg threads where the
`.repl_release: trio.Event` needs to be used from the main `trio`
thread OW confusing-but-valid teardown tracebacks can show under various
races.
Also improve,
- log reporting for such internal bugs to make them more obvious on
console via `log.exception()`.
- only restore the SIGINT handler when runtime is (still) active.
- reporting when `tractor.pause(shield=True)` should be used and
unhiding the internal frames from the tb in that case.
- for `pause_from_sync()` some deep fixes..
|_add a `allow_no_runtime: bool = False` flag to allow
**not** requiring the actor runtime to be active.
|_fix the `greenback` case-branch to only trigger on `not
is_trio_thread`.
|_add a scope-global `repl_owner: Task|Thread|None = None` to
avoid ref errors..
Various `try`/`except` blocks around external APIs that raise when not
running inside an `tractor` and/or some async framework (mostly to avoid
too-late/benign error tbs on certain classes of actor tree teardown):
- for the `log.pdb()` prompts emitted before REPL console entry.
- inside `DebugStatus.is_main_trio_thread()`'s call to `sniffio`.
- in `_post_mortem()` by catching `NoRuntime` when called from a thread
still active after the `.open_root_actor()` has already exited.
Also,
- create a dedicated `DebugStateError` for raising instead of `assert`s
when we have actual debug-request inconsistencies (as seem to be most
likely with bg thread usage of `breakpoint()`).
- show the `open_crash_handler()` frame on `bdb.BdbQuit` (for now?)
Since it seems that `pdbp.xpm()` can sometimes lose the up-stack
traceback info/frames? Not sure why but ours seems to work just fine
from a `asyncio`-handler in `modden`'s use of `i3ipc` B)
Also call `DebugStatus.shield_sigint()` from `pause_from_sync()` in the
infected-`asyncio` case to get the same shielding behaviour as in all
other usage!
Mostly due to magic from @oremanj where we slap in a little bit of
`.from_asyncio`-type stuff to run a `trio`-task from `asyncio.Task`
code!
I'm not gonna go into tooo too much detail but basically the primary
thing needed was a way to (blocking-ly) invoke a `trio.lowlevel.Task`
from an `asyncio` one (which we now have with a new
`run_trio_task_in_future()` thanks to draft code from the aforementioned
jefe) which we now invoke from a dedicated aio case-branch inside
`.devx._debug.pause_from_sync()`. Further include a case inside
`DebugStatus.release()` to handle using the same func to set the
`repl_release: trio.Event` from the aio side when releasing the REPL on
exit cmds.
Prolly more refinements to come ;{o
Just like we've started doing throughout the rest of the actor runtime
for reporting (and where "sclang" = "structured conc (s)lang", our
little supervision-focused operations syntax i've been playing with in
log msg content).
Further tweaks:
- report the `trio_done_fute` alongside the `main_outcome` value.
- add a todo list for supporting `greenback` for pause points.
The reason for this "duplication" with the `--asyncio` CLI flag (passed
to the child during spawn) is 2-fold:
- allows verifying inside `Actor._from_parent()` that the `trio` runtime was
started via `.start_guest_run()` as well as if the
`Actor._infected_aio` spawn-entrypoint value has been set (by the
`._entry.<spawn-backend>_main()` whenever `--asyncio` is passed)
such that any mismatch can be signaled via an `InternalError`.
- enables checking the `._state._runtime_vars['_is_infected_aio']` value
directly (say from a non-actor/`trio`-thread) instead of calling
`._state.current_actor(err_on_no_runtime=False)` in certain edge
cases.
Impl/testing deats:
- add `._state._runtime_vars['_is_infected_aio'] = False` default.
- raise `InternalError` on any `--asyncio`-flag-passed vs.
`_runtime_vars`-value-relayed-from-parent inside
`Actor._from_parent()` and include a `Runner.is_guest` assert for good
measure B)
- set and relay `infect_asyncio: bool` via runtime-vars to child in
`ActorNursery.start_actor()`.
- verify `actor.is_infected_aio()`, `actor._infected_aio` and
`_state._runtime_vars['_is_infected_aio']` are all set in test suite's
`asyncio_actor()` endpoint.
By re-purposing our `pexpect`-based console matching with a new
`debugging/shield_hang_in_sub.py` example, this tests a few "hanging
actor" conditions more formally:
- that despite a hanging actor's task we can dump
a `stackscope.extract()` tree on relay of `SIGUSR1`.
- the actor tree will terminate despite a shielded forever-sleep by our
"T-800" zombie reaper machinery activating and hard killing the
underlying subprocess.
Some test deats:
- simulates the expect actions of a real user by manually using
`os.kill()` to send both signals to the actor-tree program.
- `pexpect`-matches against `log.devx()` emissions under normal
`debug_mode == True` usage.
- ensure we get the actual "T-800 deployed" `log.error()` msg and
that the actor tree eventually terminates!
Surrounding (re-org/impl/test-suite) changes:
- allow disabling usage via a `maybe_enable_greenback: bool` to
`open_root_actor()` but enable by def.
- pretty up the actual `.devx()` content from `.devx._stackscope`
including be extra pedantic about the conc-primitives for each signal
event.
- try to avoid double handles of `SIGUSR1` even though it seems the
original (what i thought was a) problem was actually just double
logging in the handler..
|_ avoid double applying the handler func via `signal.signal()`,
|_ use a global to avoid double handle func calls and,
|_ a `threading.RLock` around handling.
- move common fixtures and helper routines from `test_debugger` to
`tests/devx/conftest.py` and import them for use in both test mods.
The final issue was making sure we do the same thing on ctl-c/SIGINT
from the user. That is, if there's already a bg-thread in REPL, we
`log.pdb()` about SIGINT shielding and re-draw the prompt; the same UX
as normal actor-runtime-task behaviour.
Reasons this wasn't workin.. and the fix:
- `.pause_from_sync()` was overriding the local `repl` var with `None`
delivered by (transitive) calls to `_pause(debug_func=None)`.. so
remove all that and only assign it OAOO prior to thread-type case
branching.
- always call `DebugStatus.shield_sigint()` as needed from all requesting
threads/tasks:
- in `_pause_from_bg_root_thread()` BEFORE calling `._pause()` AND BEFORE
yielding back to the bg-thread via `.started(out)` to ensure we're
definitely overriding the handler in the `trio`-main-thread task
before unblocking the requesting bg-thread.
- from any requesting bg-thread in the root actor such that both its
main-`trio`-thread scheduled task (as per above bullet) AND it are
SIGINT shielded.
- always call `.shield_sigint()` BEFORE any `greenback._await()` case
don't entirely grok why yet, but it works)?
- for `greenback._await()` case always set `bg_task` to the current one..
- tweaks to the `SIGINT` handler, now renamed `sigint_shield()` so as
not to name-collide with the methods when editor-searching:
- always try to `repr()` the REPL thread/task "owner" as well as the
active `PdbREPL` instance.
- add `.devx()` notes around the prompt flushing deats and comments
for any root-actor-bg-thread edge cases.
Related/supporting refinements:
- add `get_lock()`/`get_debug_req()` factory funcs since the plan is to
eventually implement both as `@singleton` instances per actor.
- fix `acquire_debug_lock()`'s call-sig-bug for scheduling
`request_root_stdio_lock()`..
- in `._pause()` only call `mk_pdb()` when `debug_func != None`.
- add some todo/warning notes around the `cls.repl = None` in
`DebugStatus.release()`
`test_pause_from_sync()` tweaks:
- don't use a `attach_patts.copy()`, since we always `break` on match.
- do `pytest.fail()` on that ^ loop's fallthrough..
- pass `do_ctlc(child, patt=attach_key)` such that we always match the
the current thread's name with the ctl-c triggered `.pdb()` emission.
- oh yeah, return the last `before: str` from `do_ctlc()`.
- in the script, flip `abandon_on_cancel=True` since when `False` it
seems to cause `trio.run()` to hang on exit from the last bg-thread
case?!?
In `lock_stdio_for_peer()` better internal-error handling/reporting:
- only `Lock._blocked.remove(ctx.cid)` if that same cid was added on
entry to avoid needless key-errors.
- drop all `Lock.release(force: bool)` usage remnants.
- if `req_ctx.cancel()` fails mention it with `ctx_err.add_note()`.
- add more explicit internal-failed-request log messaging via a new
`fail_reason: str`.
- use and use new `x)<=\n|_` annots in any failure logging.
Other cleanups/niceties:
- drop `force: bool` flag entirely from the `Lock.release()`.
- use more supervisor-op-annots in `.pdb()` logging
with both `_pause/crash_msg: str` instead of double '|' lines when
`.pdb()`-reported from `._set_trace()`/`._post_mortem()`.
Turns out it does work XD
Prior presumption was from before I had the fute poll-loop so makes
sense we needed more then one sched-tick's worth of context switch vs.
now we can just keep looping-n-pumping as fast possible until the
guest-run's main task completes.
Also,
- minimize the preface commentary (as per todo) now that we have tests
codifying all the edge cases :finger_crossed:
- parameter-ize the pump-loop-cycle delay and default it to 0.
To make the recent set of tests pass this (hopefully) finally solves all
`asyncio` embedded `trio` guest-run abandonment by ensuring we "pump the
event loop" until the guest-run future is fully complete.
Accomplished via simple poll loop of the form `while not
trio_done_fut.done(): await asyncio.sleep(.1)` in the `aio_main()`
task's exception teardown sequence. The loop does a naive 10ms
"pump-via-sleep & poll" for the `trio` side to complete before finally
exiting (and presumably raising) from the SIGINT cancellation.
Other related cleanups and refinements:
- use `asyncio.Task.result()` inside `cancel_trio()` since it also
inline-raises any exception outcome and we can also log-report the
result in non-error cases.
- comment out buncha not-sure-we-need-it stuff in `cancel_trio()`.
- remove the botched `AsyncioCancelled(CancelledError):` idea obvi XD
- comment `greenback` init for now in `aio_main()` since (pretty sure)
we don't ever want to actually REPL in that specific func-as-task?
- always capture any `fute_err: BaseException` from the `main_outcome:
Outcome` delivered by the `trio` side guest-run task.
- add and raise a new super noisy `AsyncioRuntimeTranslationError`
whenever we detect that the guest-run `trio_done_fut` has not
completed before task exit; should avoid abandonment issues ever
happening again without knowing!
Turns out it somehow breaks our `to_asyncio` error relay since obvi
`asyncio`'s runtime seems to specially handle it (prolly via
`isinstance()` ?) and it caused our
`test_aio_cancelled_from_aio_causes_trio_cancelled()` to hang..
Further, obvi `unpack_error()` won't be able to find the type def if not
kept inside `._exceptions`..
So given all that, revert the change/move as well as:
- tweak the aio-from-aio cancel test to timeout.
- do `trio.sleep()` conc with any bg aio task by moving out nursery
block.
- add a `send_sigint_to: str` parameter to
`test_sigint_closes_lifetime_stack()` such that we test the SIGINT
being relayed to just the parent or the child.
Took me a while to figure out what the heck was going on but, turns out
`asyncio` changed their SIGINT handling in 3.11 as per:
https://docs.python.org/3/library/asyncio-runner.html#handling-keyboard-interruption
I'm not entirely sure if it's the 3.11 changes or possibly wtv further
updates were made in 3.12 but more or less due to the way
our current main task was written the `trio` guest-run was getting
abandoned on SIGINTs sent from the OS to the infected child proc..
Note that much of the bug and soln cases are layed out in very detailed
comment-notes both in the new test and `run_as_asyncio_guest()`, right
above the final "fix" lines.
Add new `test_infected_aio.test_sigint_closes_lifetime_stack()` test suite
which reliably triggers all abandonment issues with multiple cases
of different parent behaviour post-sending-SIGINT-to-child:
1. briefly sleep then raise a KBI in the parent which was originally
demonstrating the file leak not being cleaned up by `Actor.lifetime_stack.close()`
and simulates a ctl-c from the console (relayed in tandem by
the OS to the parent and child processes).
2. do `Context.wait_for_result()` on the child context which would
hang and timeout since the actor runtime would never complete and
thus never relay a `ContextCancelled`.
3. both with and without running a `asyncio` task in the `manage_file`
child actor; originally it seemed that with an aio task scheduled in
the child actor the guest-run abandonment always was the "loud" case
where there seemed to be some actor teardown but with tbs from
python failing to gracefully exit the `trio` runtime..
The (seemingly working) "fix" required 2 lines of code to be run inside
a `asyncio.CancelledError` handler around the call to `await trio_done_fut`:
- `Actor.cancel_soon()` which schedules the actor runtime to cancel on
the next `trio` runner cycle and results in a "self cancellation" of
the actor.
- "pumping the `asyncio` event loop" with a non-0 `.sleep(0.1)` XD
|_ seems that a "shielded" pump with some actual `delay: float >= 0`
did the trick to get `asyncio` to allow the `trio` runner/loop to
fully complete its guest-run without abandonment.
Other supporting changes:
- move `._exceptions.AsyncioCancelled`, our renamed
`asyncio.CancelledError` error-sub-type-wrapper, to `.to_asyncio` and make
it derive from `CancelledError` so as to be sure when raised by our
`asyncio` x-> `trio` exception relay machinery that `asyncio` is
getting the specific type it expects during cancellation.
- do "summary status" style logging in `run_as_asyncio_guest()` wherein
we compile the eventual `startup_msg: str` emitted just before waiting
on the `trio_done_fut`.
- shield-wait with `out: Outcome = await asyncio.shield(trio_done_fut)`
even though it seems to do nothing in the SIGINT handling case..(I
presume it might help avoid abandonment in a `asyncio.Task.cancel()`
case maybe?)
As in whenever `Context.cancel()` is not (runtime internally) called
(i.e. `._cancel_called` is not set), we can attempt to detect the parent
`trio` nursery/cancel-scope that is the source. Emit the report with
a `.cancel()` level and attempt to repr in "sclang" form as well as
unhide the stack frame for debug/traceback-in.
There's a been a todo for soo long for this XD
Since all `Actor`'s store a set of `._peers` we can try a lookup on that
table as a shortcut before pinging the registry Bo
Impl deats:
- add a new `._discovery.get_peer_by_name()` routine which attempts the
`._peers` lookup by combining a copy of that `dict` + an entry added
for `Actor._parent_chan` (since all subs have a parent and often the
desired contact is just that connection).
- change `.find_actor()` (for the `only_first == True` case),
`.query_actor()` and `.wait_for_actor()` to call the new helper and
deliver appropriate outputs if possible.
Other,
- deprecate `get_arbiter()` def and all usage in tests and examples.
- drop lingering use of `arbiter_sockaddr` arg to various routines.
- tweak the `Actor` doc str as well as some code fmting and a tweak to
the `._stream_handler()`'s initial `con_status: str` logging value
since the way it was could never be reached.. oh and `.warning()` on
any new connections which already have a `_pre_chan: Channel` entry in
`._peers` so we can start minimizing IPC duplications.
In the `drain_to_final_msg()` impl, since a stream terminating
gracefully requires this msg, there's really no reason to `log.cancel()`
about it; go `.runtime()` level instead since we're trying de-noise
under "normal operation".
Also,
- passthrough `hide_tb` to taskc-handler's `ctx.maybe_raise()` call.
- raise `MessagingError` for the `MsgType` unmatched `case _:`.
- detail the doc string motivation a little more.
As per a WIP scribbled out TODO in `._entry.nest_from_op()`, change
a bunch of "supervisor/lifetime mgmt ops" related log messages to
contain some supervisor-annotation "headers" in an effort to give
a terser "visual indication" of how some execution/scope/storage
primitive entity (like an actor/task/ctx/connection) is being operated
on (like, opening/started/closed/cancelled/erroring) from a "supervisor
action" POV.
Also tweak a bunch more emissions to lower levels to reduce noise around
normal inter-actor operations like process and IPC ctx supervision.
To avoid showing lowlevel details of exception handling around the
underlying call to `return await self._ctx._pld_rx.recv_pld(ipc=self)`,
any time a `RemoteActorError` is unpacked (an raised locally) we re-raise
it directly from the captured `src_err` captured so as to present to
the user/app caller-code an exception raised directly from the `.receive()`
frame. This simplifies traceback call-stacks for any `log.exception()`
or `pdb`-REPL output filtering out the lower `PldRx` frames by default.
Since it was all ad-hoc defined inside
`._ipc.MsgpackTCPStream._iter_pkts()` more or less, this starts
formalizing a way for particular transport backends to indicate whether
a disconnect condition should be re-raised in the RPC msg loop and if
not what log level to report it at (if any).
Based on our lone transport currently we try to suppress any logging
noise from ephemeral connections expected during normal actor
interaction and discovery subsys ops:
- any short lived discovery related TCP connects are only logged as
`.transport()` level.
- both `.error()` and raise on any underlying `trio.ClosedResource`
cause since that normally means some task touched transport layer
internals that it shouldn't have.
- do a `.warning()` on anything else unexpected.
Impl deats:
- extend the `._exceptions.TransportClosed` to accept an input log
level, raise-on-report toggle and custom reporting & raising via a new
`.report_n_maybe_raise()` method.
- construct the TCs with inputs per case in (the newly named) `._iter_pkts().
- call ^ this method from the `TransportClosed` handler block inside the
RPC msg loop thus delegating reporting levels and/or raising to the
backend's per-case TC instantiating.
Related `._ipc` changes:
- mask out all the `MsgpackTCPStream._codec` debug helper stuff and drop
any lingering cruft from the initial proto-ing of msg-codecs.
- rename some attrs/methods:
|_`MsgpackTCPStream._iter_packets()` -> `._iter_pkts()` and
`._agen` -> `_aiter_pkts`.
|_`Channel._aiter_recv()` -> `._aiter_msgs()` and
`._agen` -> `_aiter_msgs`.
- add `hide_tb: bool` support to `Channel.send()` and only show the
frame on non-MTEs.
- allow passing and report the lib name (`trio` or `tractor`) from
`maybe_open_nursery()`.
- use `.runtime()` level when reporting `_Cache`-hits in
`maybe_open_context()`.
- tidy up some doc strings.
In case the struct doesn't import a field type (which will cause the
`.pformat()` to raise) just report the issue and try to fall back to the
original `repr()` version.
It's too fragile to put in side core RPC machinery since
`msgspec.Struct` defs can fail if a field type can't be
looked up at creation time (like can easily happen if you
conditionally import using `if TYPE_CHECKING:`)
Also,
- rename `cs` to `rpc_ctx_cs: CancelScope` since it's literally
the wrapping RPC `Context._scope`.
- report self cancellation via `explain: str` and add tail case for
"unknown cause".
- put a ?TODO? around what to do about KBIs if a context is opened
from an `infected_aio`-actor task.
- similar to our nursery and portal add TODO list for moving all
`_invoke_non_context()` content out the RPC core and instead implement
them as `.hilevel` endpoint helpers (maybe as decorators?)which under
neath define `@context`-funcs.
Tyler Goodlet
Guillermo Rodriguez