Skip to content

refactor(opds): sharing as an explicit state machine with Local/All network modes - #160

Merged
phildenhoff merged 1 commit into
mainfrom
rework-opds-sharing
Sep 22, 2026
Merged

phildenhoff merged 1 commit into
mainfrom
rework-opds-sharing

Conversation

@phildenhoff

Copy link
Copy Markdown
Member

Problem

The sharing service grew a reconcile loop: every 2 seconds it re-enumerated interfaces, diffed the plan against the running listeners, restarted failures with backoff, and tracked a ledger of per-address failures. That loop was the most complex code in the feature, and it existed to handle a case that barely matters for v1 (the network changing underneath a running share). Separately, the interface picker exposed ~65 entries on a developer machine (Docker bridges, Colima, Tailscale, AWDL...) — unusable noise, and the wrong abstraction.

What this does

The service is now an explicit state machine — one private enum, variants owning their resources:

  • Stopped → start → Starting → Running {listeners, urls} / Waiting {reason} / Failed {error}
  • Transitions are the only place state changes: begin_start, started(gen), waiting(gen), listener_died(gen), failed(gen), stop. Illegal transitions don't exist because no code can express them.
  • A generation counter dismisses stale events: a death watcher firing after stop, a retry landing after stop, a bind completing after stop — all no-ops. The mutex is never held across an await; a stop during an in-flight bind resolves in three lines (started(gen) sees the stale generation and drains the fresh listeners).
  • The UI status is a derived projection (From<&SharingState>), so state and reported status cannot drift.

The loop is gone. Interfaces are enumerated once per attempt. Waiting runs a small poll that exists only while waiting (sharing auto-starts when a network appears; everything else is turn-it-off-and-on). A dead listener fails the service with honest copy — "Sharing stopped unexpectedly. Turn it back on." Stop is awaited (5s budget); stale events can't resurrect anything.

Two bind modes instead of a picker:

  • localNetworks (default): classified LAN interfaces, private/ULA addresses, exact binds, never Docker bridges/Tailscale/global addresses.
  • allInterfaces: wildcard 0.0.0.0 + [::] with IPV6_V6ONLY set explicitly (Linux dual-stack defaults differ), no enumeration at all. Intended to be gated on username/password once auth lands (feat(opds): HTTP Basic auth and credential management #149); the BindPolicy hook (allow_global) is already threaded for that.

The per-interface target and the interfaces query are deleted (bindings regenerated). Globally-routable addresses are excluded from local-network binds by default.

Testing

40 tests. The state machine's transitions are table-tested purely (no sockets): stale generations rejected, stop-from-any-state, derived status. Service tests cover: conflict on double-start, waiting→poll→running, stop cancelling the poll, dead listener → failed → restart, permanent bind failure, AllInterfaces binding without enumeration, and a real dual-wildcard bind on one port (the socket2 path).

Notes

@github-actions

Copy link
Copy Markdown

libcalibre Test Coverage Report

Overall coverage: 80.16%

📊 Download HTML Report

Coverage breakdown available in the artifacts.

…etwork modes

The service becomes a small enum state machine - Stopped / Starting /
Running / Waiting / Failed - whose variants own their resources, with
transitions as the only place state changes and a generation counter that
dismisses stale events (a death watcher firing after stop, a retry landing
after stop). The mutex is never held across an await: begin_start bumps the
generation, the bind happens unlocked, and started(gen) reconciles.

The monitor/reconcile loop, failure ledger, and backoff ladder are gone:
interfaces are enumerated once per attempt, Waiting retries on a small poll
that exists only while Waiting, and a dead listener fails the service with
honest copy (turn sharing back on).

Bind targets become LocalNetworks (classified LAN, private/ULA, policy-
filtered) and AllInterfaces (0.0.0.0 + :: with V6ONLY set, the future auth-
gated mode). The per-interface picker target is deleted along with the
interfaces query - the UI will offer two choices, not sixty-five.
@phildenhoff

Copy link
Copy Markdown
Member Author

Plato's implementation review flagged three real issues, all fixed in this push:

  1. Generation counter is now controller-level monotonic (AtomicU64 on the service, never derived from the state variant) — closes the start→stop→start race where a stale bind completion could match the new run's generation.
  2. SO_REUSEADDR on the socket2 path (unix) — tokio's bind sets it; a hand-built socket doesn't. Without it, restarting sharing within TIME_WAIT of a client connection failed with a spurious permanent "port in use". New test: bind → client connects → server-side close → immediate rebind must succeed.
  3. AllInterfaces is now gated at the service boundary — without credentials configured (allow_global false, the only value main can produce until feat(opds): HTTP Basic auth and credential management #149) it returns AuthRequired rather than silently serving every network. Nightlies build from main, so the guard ships now and feat(opds): HTTP Basic auth and credential management #149 just flips the input.

Plus two smaller items: Listeners::drop now signals graceful shutdown and detaches instead of aborting (aborting killed the serve future before it could process the signal, leaving keep-alive connections serving); and double-start with an identical config is an idempotent no-op (UI double-click) while a different config still conflicts. Running carries its config, so state and reported config can't drift.

@github-actions

Copy link
Copy Markdown

libcalibre Test Coverage Report

Overall coverage: 80.16%

📊 Download HTML Report

Coverage breakdown available in the artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant