fix(whip): evaluate the inactivity watchdog per poll tick, not per timeout

[wagenet]

The bug

backend/src/blocks/builtin/whip.rs — the WHIP inactivity watchdog slept a full
INACTIVITY_TIMEOUT (10s) and then evaluated idle time exactly once:

let deadline = Instant::now() + INACTIVITY_TIMEOUT;   // fresh 10s every iteration
if wait_until_deadline_or_stop(&cleanup_sent_watchdog, deadline) { break; }

A drop landing just after a check went unnoticed until the next one, so real detection
was 10-20s depending on where the drop fell in the cycle. A production
Inactivity timeout (17s idle) is only possible because of that coarseness — with
continuous evaluation the reported idle can never exceed the threshold by more than one
poll interval.

Why it matters

This watchdog is the only time-driven path that reclaims a WHIP session, and a publisher
that dies without sending a WHIP DELETE — network loss, the common case — reaches no
other one: an abruptly killed peer leaves webrtcbin at completed until consent
freshness expires. Until it fires, the session's isolated pipeline (the libnice #52
workaround) holds its UDP sockets, threads and jitterbuffers; the slot stays occupied, so
a rejoining client gets 503 All session slots are occupied (max_sessions is 1 per
endpoint); and the API keeps reporting a publisher that is gone.

Relationship to the slot-takeover work

A separate change lets a new session displace a slot whose transport is already dead
(whip_ingest.rs allocate_slot). It covers the rejoin case and makes the 503 window
moot, but it is demand-driven and never runs when nobody reconnects — which is where this
watchdog stays the only reclamation path.

Takeover must decide a session is dead and then evict it, and a wrong predicate evicts a
healthy publisher during a transient stall, precisely what the 10s threshold exists to
prevent. That change can misfire; this one cannot, since it only makes an already-correct
teardown happen on schedule. That argues for landing this first.

The fix

Extract the loop into wait_for_inactivity, which re-evaluates idle on every
WATCHDOG_POLL (250ms) tick — a constant that already existed in this file and was used
only to re-check the stop flag inside the wait.

INACTIVITY_TIMEOUT is deliberately unchanged at 10s: it guards against tearing down a
session during a brief network stall. The bug was that 10s silently meant up to 20s.

The log line now reports idle in milliseconds rather than truncated seconds. Nothing
parses that string.

Measured

A headless whipclientsink publisher, SIGKILLed to simulate network loss — a clean
SIGINT sends the WHIP DELETE and frees the slot instantly, which is not the case under
test. Media flow was confirmed by the Pad video_0 ... → appsink → slot 0 appsrc link
line, not by ICE state. Dwell is seconds of streaming before the kill, sweeping the drop
across a full watchdog cycle; the figure is the idle time the watchdog itself reports.

dwell	before	after
0s	19198ms	10076ms
2s	17165ms	10058ms
4s	15176ms	10206ms
6s	13145ms	10219ms
8s	11187ms	10222ms
10s	19188ms	10229ms

Before is a sawtooth from 19.2s down to 11.2s, wrapping — the production 17s sits on it.
After is flat at 10.06-10.23s, every sample within one WATCHDOG_POLL of the threshold.
Eight consecutive kill/rejoin cycles gave 10006-10197ms with zero 503s, each
re-allocating slot 0.

Tests

watchdog_detects_inactivity_within_one_poll_of_the_timeout calls the real
wait_for_inactivity with the production WATCHDOG_POLL. It is a guard, not a
demonstration — reverting the one-line change and re-running gives:

inactivity took 2.015019458s to detect with a 1s timeout — idle is being
evaluated once per timeout, not once per poll

watchdog_inactivity_wait_gives_up_when_stopped covers the other exit: a teardown path
setting cleanup_sent must end the wait even though the session never went idle,
otherwise the watchdog outlives its session and asks the manager to clean up a port it no
longer knows.

Both are plain std timing with no GStreamer element, so they run in CI under
cargo test --package strom.

Ran: cargo test --package strom (543 lib + all integration tests, passing),
cargo fmt --all, cargo clippy --all-targets, plus the measurements above on macOS.
Not run: the Linux CI matrix.

Scope

Watchdog only — no change to allocate_slot or the session manager, so this stays
independently reviewable from the takeover work.

🤖 Generated with Claude Code

[srperens]

Claims

Claim	Verdict	Evidence
The watchdog now re-evaluates idle time every WATCHDOG_POLL (250ms) tick instead of once per INACTIVITY_TIMEOUT (10s) cycle	CONFIRMED	backend/src/blocks/builtin/whip.rs:552 — if wait_until_deadline_or_stop(stop, Instant::now() + WATCHDOG_POLL) { inside the new wait_for_inactivity loop, replacing the old per-iteration Instant::now() + INACTIVITY_TIMEOUT deadline
INACTIVITY_TIMEOUT itself is unchanged at 10s — only the poll granularity changed	CONFIRMED	backend/src/blocks/builtin/whip.rs:506 — const INACTIVITY_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); — untouched by the diff
A regression test fails against the old once-per-timeout behaviour and passes against the fix	CONFIRMED	backend/src/blocks/builtin/whip.rs:1911 — fn watchdog_detects_inactivity_within_one_poll_of_the_timeout() calls the real wait_for_inactivity and asserts detection under 1600ms for a 1000ms timeout, which a once-per-timeout evaluation (≈2000ms) would fail
The stop path still exits promptly when a teardown claims the session before it goes idle	CONFIRMED	backend/src/blocks/builtin/whip.rs:1942 — fn watchdog_inactivity_wait_gives_up_when_stopped() — asserts result.is_none() and completion under 5s for a 30s timeout
Only one production call site needed updating	CONFIRMED	backend/src/blocks/builtin/whip.rs:804 — let Some(idle_ms) = wait_for_inactivity( is the sole non-test caller (grep for wait_for_inactivity shows no others)

Diagnosis — this is the right fix, not a workaround. The bug was a granularity mismatch, not a wrong threshold: sleeping a full INACTIVITY_TIMEOUT and checking once means a drop landing just after a check waits almost two timeouts to be noticed, which matches the PR's measured 11.2-19.2s sawtooth against a 10s threshold. Re-evaluating every poll tick bounds detection to timeout + one poll, which the "flat at 10.06-10.23s" measurement confirms. INACTIVITY_TIMEOUT staying at 10s is correct — that value guards against tearing down a session during a transient stall, and was never the actual bug.

Radius — LOCAL: wait_for_inactivity is new, private (fn, not pub), and has exactly one production caller in the same file. No change to allocate_slot or the session manager, matching the PR's stated scope; the companion slot-takeover work is explicitly independent of this change.

Tests & CI — Build (Linux x86_64/ARM64), Check (Linux), Check & Build (WASM), API Contract Check, sccache preflight all pass at c267132. Check (Linux) runs cargo test --package strom (.github/workflows/ci.yml:129), which executes both new tests — neither needs a GStreamer element (std timing only), so nothing here is CI-skipped. Build (macOS)/(Windows) skip as usual for this repo. The PR body's own manual SIGKILL/rejoin measurements are the author's claim, not something I ran or can verify independently — noted as author-reported, not evidence.

No CLAUDE.md violation: no pad probe, no new #[allow(dead_code)], no emoji in log macros, and the log line's time unit changed from truncated seconds to milliseconds with a comment noting nothing parses that string.

Confidence: HIGH