Skip to content

Accept the second screen step as where a session-locked loop is stuck - #222

Draft
whoisaldo wants to merge 1 commit into
release/0.3.5-candidatefrom
fix/transport-stall-phase
Draft

whoisaldo wants to merge 1 commit into
release/0.3.5-candidatefrom
fix/transport-stall-phase

Conversation

@whoisaldo

Copy link
Copy Markdown
Member

What and why

transport_stall.rs's a_stuck_transport_loop_is_named_with_its_step failed once in candidate macOS CI with [Stalled { phase: SecondScreen, silent_ms: 2009 }], where it expected Phase::Tick.

The test holds shared.session and waits for the loop to be named stuck. The loop takes that lock in two places on every tick:

  • at the tick itself (session.lock().tick(…), marked Tick);
  • after the tick's replies, at the second screen's step: second_screen::next_step calls shared.client_heard(), marked SecondScreen.

Both were already there before #192 and #220. A hold that lands between them leaves the loop stuck in its second screen step, and the watch names it correctly. That's product behaviour, not a bug: both holds of the lock are brief, and nothing in the step holds it longer.

Change

The test now takes either step that locks the session as the stuck one. It still requires exactly one stall of at least 2 s, then a Moving verdict from the same step with stalled_ms >= 2000. No product change.

How it was tested

  • With the lock taken as the loop marks SecondScreen (a local probe, not committed), the old test fails as in CI with Stalled { phase: SecondScreen, silent_ms: 2059 }. The new test passes 3 of 3 with the probe.
  • Without the probe: the old test passed 20/20 locally on the candidate, and the new one 20/20 (macOS, load 12–18).
  • cargo clippy -p eternal-host --all-targets -- -D warnings: clean.

a_stuck_transport_loop_is_named_with_its_step holds the session lock and
expected the loop to be named stuck at its tick. The loop takes that lock at
the tick and again, after the tick's replies, at the second screen's step
(client_heard in second_screen::next_step). A hold that lands between the
two leaves the loop stuck in SecondScreen, and the watch rightly says so.
It happened once in candidate CI.

The test now takes either step that locks the session, and still requires
one stall of at least 2 s, then a move from the same step.

Holding the lock as the loop marks SecondScreen, the old test fails as in
CI: Stalled { phase: SecondScreen, silent_ms: 2059 }. The new one passes.
@whoisaldo whoisaldo added the 0.3.5 After 0.3.4 label Nov 8, 2026

This branch has not been deployed

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

Labels

0.3.5 After 0.3.4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants