Repository navigation
fix: honor Event(internal=True) and give internal events from outside a step the external contract - #668
Merged
Conversation
(cherry picked from commit e633508) Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
With several external events queued, both engines took them one after the other and only then ran the internal events and eventless transitions they had enabled, so an internal event raised while handling the first external event ran after the second. SCXML's main event loop, and the processing model docs, run the macrostep after every external event. The external-queue step now takes at most one due event and returns to the macrostep. Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
Honoring Event(internal=True) at trigger time exposed how the internal
queue treated a trigger that no running step had raised:
- on the async engine, `await sm.go()` on a fresh machine was popped
before `__initial__` entered the initial state, and silently lost;
- the caller got `None` instead of the callback result, no
`TransitionNotAllowed` on a StateMachine, and no future to await;
- a delay was ignored, so `raise_("x", delay=50)` fired at once.
The internal queue now receives an internal trigger only when the
machine raises it while running a step. From outside there is no
macrostep to join, so the trigger goes to the external queue, which
already returns results, routes exceptions and orders `__initial__`
first. The context variable that marks a running step moves from the
async engine to the base, wraps both loops and the sync initial entry,
and decides the queue in `BaseEngine.put`.
A delay can never be internal: a step takes no time, and SCXML forbids
`delay` with target `#_internal`. `Event.__new__` rejects the
combination with InvalidDefinition, which covers declarations,
`raise_(delay=)`, `send(delay=, internal=True)` and `<send>` alike.
Refs #655, #663.
Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
The queue table, the raise_() and delayed-event sections and the async futures note now say that an internal event joins the macrostep in progress, goes to the external queue when triggered from outside a callback, and cannot carry a delay. The 3.2.2 notes describe the fixes behind it. Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #668 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 52
Lines 5651 5672 +21
Branches 888 891 +3
=========================================
+ Hits 5651 5672 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… task The context variable that marks a running step held a bool, so a callback of one machine that raised an internal event on another put it on the other machine's internal queue, and the caller got None. It now holds the engine running the step, and `put` honors `internal` only for that engine. Tests pin why a context variable is the right primitive: a coroutine sending while another holds the async loop gets its own result, a thread raising while another thread runs the sync loop queues behind the events it sent before, and a callback raising on another machine gets that machine's result. Each fails against the alternative it rules out (a bool flag, or the engine's lock). Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…de internal A `<send>` to `#_internal` with `delay` or `delayexpr`, in SCXML or in the native JSON/YAML format, loaded fine and then raised InvalidDefinition from the middle of a transition. It is now rejected when the action is built, through the same message as `Event`, and the JSON Schema rejects it too, so `validate=True`, the parser and the runtime agree. `send()` took `internal` as `internal or <declaration>`, so an explicit `internal=False` could not override an event declared internal, despite the docstring. It now defaults to None and falls back to the declaration only when omitted. Also pins the cross-machine contract on a StateMachine: a callback triggering a disallowed internal event on another machine raises TransitionNotAllowed. Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
…ep cost Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
Signed-off-by: Fernando Macedo <fgmacedo@gmail.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Por quê
#663 makes
Event(..., internal=True)reach the queue (#655). With it applied, a declared-internal event triggered from outside a callback returnedNone, was lost on a fresh async machine, andraise_(delay=50)fired at 0 ms (script below, 4 of 4 checks wrong on develop + #663). Closes #655.O que muda
raise_(),send(internal=True),Event(internal=True)) joins the internal queue only when raised from a callback while the machine runs a step. Triggered from outside, it goes to the external queue: the caller gets the callback result, the exception, orTransitionNotAllowedon aStateMachine, and the async caller gets a future.await sm.go()on a fresh async machine with a declared-internalgonow runs after the initial state is entered instead of being dropped.delaytogether withinternalraisesInvalidDefinition, whether declared (Event(..., delay=50, internal=True)), sent, raised (raise_(delay=50)) or loaded from an SCXML/JSON/YAML<send target="#_internal" delay=...>(rejected at load time, and by the JSON Schema). SCXML forbids the combination; previously the delay was silently ignored. An explicitsend(..., internal=False)overrides aninternal=Truedeclaration.ContextVarholding the engine, so it is right per thread, per asyncio task and per machine: a coroutine or thread sending while another runs the loop, or a callback raising on another machine, gets the external contract.ec8ce36).Evidência
evidence.py, run withuv run python evidence.pyon each commit:New tests:
tests/test_internal_triggers.py(46 cases, both engines viasm_runner) andtests/test_event_ordering.py::test_what_an_external_event_enables_runs_before_the_next_external_event. W3C SCXML suite passes on both engines.Não verificado
--benchmark-autosave) not run. A review probe measured 2000 queued external events with one guarded eventless transition: about 27-31% slower (51 ms vs 40 ms sync), since eventless transitions are now checked after every external event.thread_inherit_context): a thread spawned from inside a step would inherit the "running a step" flag; only GIL builds were run._event.typein SCXML still reports"internal"for a declared-internal event triggered from outside, even though it is queued externally.