Harden runtime correctness and performance - #45
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10b6d3f524
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Match XState: a stopped actor does not restart. | ||
| return self | ||
| effective = initial_state if initial_state is not None else self._snapshot | ||
| self._backend.start(cast(Any, effective)) |
There was a problem hiding this comment.
Release the lifecycle lock before starting actor logic
When an invoked from_callback implementation starts a worker, waits for it, and that worker calls send_back, startup deadlocks: the parent holds _lifecycle_lock throughout invocation reconciliation and child.start(), while the worker blocks in parent.send() waiting for the same lock. This callback pattern completed before this change, so lifecycle state should be reserved under the lock without holding it across backend startup or other user-provided actor logic.
AGENTS.md reference: AGENTS.md:L70-L71
Useful? React with 👍 / 👎.
| current_task = asyncio.current_task() | ||
| pending = [task for task in tasks if task is not current_task] | ||
| if pending: | ||
| await asyncio.gather(*pending, return_exceptions=True) |
There was a problem hiding this comment.
Avoid cancelling the task that is calling stop
When await service.stop() is called from an action reached through a delayed send, and at least one other timer is pending, the current delayed-send task is cancelled here and the cancellation is delivered at the new await asyncio.gather(...). Consequently stop() never returns normally and the remainder of the user's action is skipped; excluding the current task only from pending is insufficient because it was already cancelled. Exclude the current task from cancellation as well.
AGENTS.md reference: AGENTS.md:L68-L69
Useful? React with 👍 / 👎.
Summary
This PR fixes confirmed runtime lifecycle, snapshot isolation, timer resource,
and transition performance defects found during a fresh audit of master at
18d6ea972a1dafb4cae29af6b2c6d1aaaf38b3ef.It preserves the public
Machine(config, ...)boundary, run-to-completionordering, immutable structural snapshot fields, XState-shaped configuration,
and zero runtime dependencies.
What changed
Correctness and lifecycle
context types such as immutable dataclasses.
mutating the derived
state.valuecannot change execution.running child.
a child spawned after its parent stopped.
restart and later events remain dropped.
occurs during an awaited async action.
Performance and resources
interpreter inspector requests them.
state.configurationinstead of rebuilding configurationfrom
state.value.threading.Timerper timeout with one on-demand daemon schedulerthread per
ThreadClock. The worker exits after the final timer fires or iscanceled.
Typing and maintainability
and the unreleased changelog.
Compatibility
configuration boundary changes.
ordering.
Code that intentionally restarted the same interpreter must create a new
interpreter instead.
ThreadClockstill gives each clock its own worker; it does not introduce aprocess-global scheduler.
finish in its caller's send task, but later runtime actions are skipped.
Performance evidence
The temporary standard-library-only harness ran outside the repository on
macOS without coverage instrumentation. Each result aggregates seven fresh
processes with three warmups and 21 samples per process. It records median,
p95, process median absolute deviation, operations per second, peak
tracemallocmemory, and task or thread counts. Each case verifies its expectedsnapshot before timing.
Representative median results:
The snapshot round-trip regression is intentional and material. The old path
returned aliased data and restored shallow context copies, so its timing did
not include the isolation promised by persistence. The new path performs the
required defensive copies. That cost scales with context size and is documented
rather than hidden behind a score.
Profiling after the change confirms that default context
deepcopyremains thedominant cost for large mutable dictionary contexts. Exit-set computation is
now once per enabled transition during conflict filtering, trace-only context
copies are absent from ordinary transitions, and 50 real timers use one worker.
These are local macOS measurements, not a service SLO or competitor ranking.
Ubuntu CI proves functionality, not equivalent performance. Cross-platform
performance remains unverified.
The harness and JSON profiles remain in
/private/tmpand are not tracked.Generated
dist/artifacts are ignored and are not part of this PR.Validation
Exact branch commit:
10b6d3f5248cba365c70bb501c70955e398b79a9poetry run python scripts/release_preflight.py v0.7.1 --target-ref HEAD --master-ref HEADpoetry run mypy --strict src/xstate/passed for all 23 source files.poetry run python -m pytest tests/ --ignore=tests/test_scxml.py --cov --cov-report=termpassed with 91.34 percent branch coverage against the 90 percent gate.
dependencies and passed a live actor transition and snapshot serialization
smoke test.
on CPython 3.13.1 and 3.14.7 at the exact branch commit.
git diff --checkpassed, added lines are ASCII, and the worktree was cleanbefore push.
Risk and rollback
The highest-risk changes are actor lifecycle locking, the shared timer worker,
terminal interpreter stop behavior, and conflict filtering. Targeted regression
tests cover their failure mechanisms, and the complete SCXML suite protects
run-to-completion behavior.
If rollback is required, revert this single commit. There are no data migrations,
new dependencies, publication changes, or external state changes.
Review focus
reconciliation.
they are absent.