fix(cmux-tui-core): viewer sizes go behind the input port's held keys without waiting; the settle ends by its deadline; a lost taken frame may be written (R-DAEMON-STATE S1.6c part 2)

Merged
LA
2d ago
+351-31

Summary

R-DAEMON-STATE stage S1 step 6c, part 2 (handoff/rewrites/R-DAEMON-STATE/DESIGN.md, section 3). Behind the per-mux switch CMUX_TUI_RDS_INPUT (default off; S1.7 turns it on). It closes the remaining review items of #35082 and #35280 before S1.7.

The defects (lane on).

  • #35082 review 1 item 4: a key that moves the grid sends a ViewerSize on its own turn, under the global sizing lock (apply_terminal_grid -> resize -> send_viewer_size). That frame waited in the input port's order barrier for up to 2 s behind the port's frames (a slow paste) while it held the sizing lock, and then it went first anyway.
  • #35082 review 2 should-do 2: an attach that settles the host's viewer size waited the barrier's 2 s, past its own deadline.
  • #35082 review 2 should-do 3: a Written frame whose writer took it and then was lost (a panic during the write) answered "not written", so the creating-input flush dropped bytes that may be on the host.

The fix.

  • ViewerSize and ReleaseViewer go on the input port when it holds frames (PortOrder::enqueue_after_pending): after those frames, with no waiter and no wait. A newer size replaces a queued one that is still the last frame, so sizes cannot pile up behind a stalled port. When the port holds nothing (or the lane is off: the port then holds nothing), they are written directly, as before.
  • begin_viewer_size_barrier takes the attach's deadline (direct_writer_until), so the settle ends by its own deadline.
  • A frame the writer took and lost answers InputMayBeWritten; one still queued answers "not written".
  • (#35375 review) A size also queues while the writer waits for a reconnect. Written directly then, it went to the lost connection, and a size queued before the loss reached the new host after the install's replay of the newest lease: the host kept the stale size (or a lease the daemon had released), and a later send of the same lease wrote nothing. Queued, the newest size replaces the trailing one, so the new host's last size frame is the newest lease or release. The barrier's reconnect skip exists because its callers hold the runtime lock the install needs; this enqueue never waits.
  • reconnect_wait reads the port state its caller holds and never takes the port lock again. The first version of enqueue_after_pending called it under the port lock, and in test builds it locked the same mutex again (the test override is cfg(test)), so the test hung. Release builds never took that second lock.
  • From the #35280 review: tests that drive the real call sites (the reader's backoff give-up wait_or_fail, and Surface::disconnect_for_daemon_shutdown) while a Written write waits for a reconnect; the reply-timeout tests take their frame on a running writer, not a new thread (#35082 review 2 should-do 4); the docs state the lock rule the right way round (the port lock is a leaf).

The person's keys (InputReply::Accepted) still never wait for a write.

Testing

Focused command: ./scripts/verify-cmux-tui-hosted.sh --filter hosted (Linux test (linux)).

Compile: fleet job d5fb95c05f9bedac13aaf0ca at the head 01af275609c: CmuxNext, app and cmux-tui passed (also 1abd4a080106318b89e00eb5 at 3bafe07e51f and c507ba1785de9898784039bf at edc0dfbbfae).

Observed / Not run

  • Observed: the hosted runs and fleet steps above. lint (linux) passed at 3bafe07e51f and at the head (at edc0dfbbfae it was red only on the base finding cmux-tui/src/cloud_environment.rs:22, from #35133).
  • Not run: macOS execution of the focused tests. The S1.7 typing A/B runs on a build that contains this change with the review fix (S1.7 head 0678016998d, build f8685e3d4f92deb22eb9a313).

Changelog

none

Proof

No capture: daemon-only change behind a default-off switch, no UI.

Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

Summary by cubic

Fixes three remaining review items for the input port lane: viewer-size frames no longer wait behind the keys the port holds, the viewer-size settle ends by its own deadline, and a frame the writer took and lost answers "may be written" so the creating-input flush keeps its bytes.

  • ViewerSize and ReleaseViewer are now queued on the input port behind its pending frames without waiting; a newer size replaces a queued one, so sizes cannot pile up. When the port holds nothing they are written directly as before. This also holds while the writer waits for a reconnect, so the new host's last size frame is the newest lease (or release) rather than a stale Queued size.
  • The viewer-size settle barrier now uses the attach's deadline (direct_writer_until) instead of the order barrier's 2 s wait.
  • A Written frame whose writer took it and was lost answers InputMayBeWritten; one still queued answers "not written".
  • reconnect_wait now reads the port state its caller already holds, fixing a test-only self-deadlock in enqueue_after_pending (release builds never took the second lock).
  • Adds tests driving the real call sites (reader's backoff give-up and daemon shutdown detach) and starts the reply-timeout tests on a running writer, not a new thread.