Skip to content

feat: add an explicit shutdown API for lanes and the auto-transition timer - #109

Merged
bagowix merged 1 commit into
mainfrom
feat/explicit-shutdown
Aug 1, 2026
Merged

feat: add an explicit shutdown API for lanes and the auto-transition timer#109
bagowix merged 1 commit into
mainfrom
feat/explicit-shutdown

Conversation

@bagowix

@bagowix bagowix commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

A coordinated breaker owns background work that had no deterministic end. The lane — a daemon thread for a sync Storage, an asyncio task for an AsyncStorage — exited only once the coordinator was garbage collected and the next tick observed a dead weakref. The auto_transition timer was cancelled when the breaker left OPEN, never on teardown. So a service dropping a breaker could not flush queued shared writes, could not know when its threads were gone, and an async lane outliving asyncio.run() produced "task was destroyed but it is pending".

This adds CircuitBreaker.close() / aclose() and Registry.close_all() / aclose_all(), over SyncCoordinator.shutdown() / AsyncCoordinator.shutdown().

Shutdown drains queued writes in FIFO order, wakes a parked lane through a sentinel op rather than waiting out poll_interval, joins the thread or awaits the task, and cancels the timer without arming another. It is idempotent and safe to call from any thread. The weakref path stays as the safety net for abandoned breakers.

Decisions the issue left open

  • Flush policy: drain. Ops already queued run before the lane exits, so a trip recorded just before shutdown still reaches the backend. Writes the degraded gate would drop are still dropped.
  • Shutdown is terminal — the lane does not restart. Afterwards the breaker keeps protecting calls on local state and drops shared writes, exactly as while the storage is degraded. Registry.close_all() keeps the cache, so get() returns the same torn-down instance instead of silently starting a fresh lane.
  • close() / aclose() follow the runtime-matching contract already enforced on call(): calling the wrong one on a coordinated breaker raises InterlockError rather than silently leaving the lane running.
  • No context-manager support. with breaker: is already the protected block; contextlib.closing(breaker) covers the need.

One thing found while implementing

Dropping the lane without dropping the cached shared view leaves the breaker wedged: nothing refreshes the cache any more, so a peer's OPEN would never expire and every call would be rejected forever. close() therefore clears the view after the join (so no in-flight write can re-adopt it) and falls back to local state, reported through on_state_change like any other transition. Covered by test__close__on_an_adopted_shared_open__falls_back_to_local_state.

Also fixes #97

Both lanes pulled an op off the queue before resolving the weak reference, so a lane stopping on a collected coordinator never called task_done(): unfinished_tasks stayed positive forever and wait_idle() could never return. Both drop paths now release the dequeued op. The issue rates this Low because nothing outside a test could reach the hang — the shutdown API is exactly what makes it reachable, which is why the two ship together.

Tests

New tests/test_shutdown.py (24 tests) runs real lane threads and tasks with poll_interval = 3600.0, so shutdown has to wake the lane through the sentinel rather than wait it out. Covers: lane stopped and joined (sync + async), drain with writes in flight, shutdown while degraded, double shutdown, shutdown before the lane started, terminal semantics, timer cancelled and never re-armed, runtime mismatch, registry close-all, fallback to local state, and the task_done() drop-path invariant with join() guarded by a timeout.

542 passed, 2 skipped, coverage 100.00%.

Checklist

  • Tests added or updated (suite stays at 100% coverage)
  • uv run ruff format --check and uv run ruff check pass
  • uv run mypy and uv run pyright pass
  • Docs updated (docs/) for user-facing changes
  • CHANGELOG.md [Unreleased] updated
  • Commits follow Conventional Commits

Related issues

Closes #98
Closes #97

…timer

A coordinated breaker owns background work that had no deterministic end.
The lane -- a daemon thread for a sync storage, an asyncio task for an
async one -- exited only once the coordinator was garbage collected and
the next tick observed a dead weakref, and the auto_transition timer was
cancelled only when the breaker left OPEN, never on teardown. A service
dropping a breaker could not flush queued shared writes nor know when its
threads were gone, and an async lane outliving asyncio.run() produced
"task was destroyed but it is pending".

Adds CircuitBreaker.close()/aclose() and Registry.close_all()/aclose_all()
over SyncCoordinator.shutdown()/AsyncCoordinator.shutdown(). Shutdown
drains the queued writes in FIFO order, wakes a parked lane through a
sentinel op instead of waiting out poll_interval, joins the thread or
awaits the task, and cancels the timer without arming another. It is
idempotent and safe to call from any thread.

Two semantics worth stating. Shutdown is terminal: the lane never
restarts and later shared writes are dropped, exactly as while the
storage is degraded, so the breaker keeps protecting calls on local
state. And the cached shared view is dropped with the lane -- nothing
refreshes it once the lane is gone, so retaining it would pin the breaker
in whatever a peer last published and a shared OPEN would never expire.
The fallback surfaces through on_state_change like any other transition.
close() is teardown, not a state change: it does not close the circuit.

The weakref path stays as the safety net for abandoned breakers.

Fixes #98

Also fixes the queue invariant that made this reachable (#97): both lanes
pulled an op off the queue before resolving the weak reference, so a lane
stopping on a collected coordinator never called task_done() and
unfinished_tasks stayed positive forever -- wait_idle() could not return.
Both drop paths now release the dequeued op.

Fixes #97
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  interlock
  _coordination.py
  _engine.py
  breaker.py
  registry.py
Project Total  

This report was generated by python-coverage-comment-action

@codspeed-hq

codspeed-hq Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 21 untouched benchmarks


Comparing feat/explicit-shutdown (20cc334) with main (f0ca0ef)

Open in CodSpeed

@bagowix
bagowix merged commit 80c165c into main Aug 1, 2026
12 of 13 checks passed
@bagowix
bagowix deleted the feat/explicit-shutdown branch August 1, 2026 14:47
bagowix added a commit that referenced this pull request Aug 1, 2026
…on (#110)

## Summary

Two gaps in `AGENTS.md` that have each cost rework already.

**1. The CHANGELOG and docs obligations lived only in the PR
checklist.** They surface at review time rather than while the work
happens, so a change could arrive at a PR complete in every respect
except the two things a user actually reads. `#108` shipped its
`CHANGELOG` entry only because the template got opened at the end.

Adds a **Definition of done** section naming them explicitly, together
with the `llms-full.txt` regeneration step (previously documented only
in `docs/CLAUDE.md`, which an agent working in `interlock/` never reads)
and the tests-first expectation. The PR template is called out as the
last gate, not the first reminder.

**2. The "no silent exceptions" hard rule now has a deliberate, shipped
counter-example.** `interlock/_notify.py` logs a raising `EventListener`
hook with its traceback and swallows it, so observability cannot replace
a protected result, mask a dependency's exception, or kill a coordinator
lane (#83). Without that written down, the next reader — human or agent
— sees a blind `except Exception` in a codebase whose style guide
forbids exactly that, and "fixes" it.

The note records the exception, states why it is not actually silent
(`BaseException` still propagates, the traceback is logged), and bounds
it so it is not read as licence to swallow anywhere else.

## Checklist

- [ ] Tests added or updated (suite stays at 100% coverage) — n/a,
documentation only
- [x] `uv run ruff format --check` and `uv run ruff check` pass
- [x] `uv run mypy` and `uv run pyright` pass
- [x] Docs updated (`docs/`) for user-facing changes — n/a, `AGENTS.md`
is contributor-facing
- [ ] `CHANGELOG.md` `[Unreleased]` updated — n/a, no user-visible
behaviour change
- [x] Commits follow Conventional Commits

## Related issues

None — follow-up housekeeping after #108 and #109.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant