Skip to content

Release held input when a client goes silent - #66

Draft
TechLuddite wants to merge 9 commits into
omacom:mainfrom
TechLuddite:fix/server-heartbeat
Draft

TechLuddite wants to merge 9 commits into
omacom:mainfrom
TechLuddite:fix/server-heartbeat

Conversation

@TechLuddite

@TechLuddite TechLuddite commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

The server noticed EOF and write errors but not a link that goes silent. It sent one Ping at startup, the session loop had no read deadline, and gliff configures no ssh keepalive. A client that slept or lost its network with a key held left the key pressed on the server, and remote apps repeated it until the transport failed, with no fixed bound.

The session now pings every 2 s and treats any client message as liveness. When nothing arrives within 10 s of a Ping finishing its write (30 s while writes are blocked), the server releases every held key and button. Caps Lock and the layout stay as they were, so they still match the client when it comes back. Key and pointer input is then dropped until the client answers a Ping sent after the release. Each message is tagged with the input epoch it arrived in, so input received while the client was lost, or received before the release but handled after it, is never injected. A client that stays silent ends the session through the normal loop exit, so the ordered teardown runs: after 5 minutes on a headless output, which a reconnect cannot reattach, or after 60 s on a mirrored one.

The reader, the writer and the session loop share one heartbeat on the same LocalSet. The reader applies each message and Pong as it reads it, and the writer records each Ping when its write completes, so a backlog of slow handlers cannot hide a client that came back. The writer queue now accepts and coalesces Ping, and gliff-probe answers Ping while streaming, as the client does. The protocol is unchanged, and clients since 0.3.0 already answer every Ping.

Validation:

  • Live on an RTX 3090 desktop (mirrored 2560x1440 output, Hyprland), with this branch's server and gliff-probe stream-bench on a laptop over an ssh tunnel, freezing the client with SIGSTOP. Input was released 11 s after the freeze and accepted again within a second of resuming, and streaming continued. Left frozen, the session ended 60 s after the release.
  • Unit tests drive the Heartbeat state machine with an injected clock. Session tests cover release on silence, input dropped while lost, stale Pongs and stale input, the reader applying messages as it reads them, and both grace periods. The deadline wait the session loop uses runs on tokio's paused clock in its own test.
  • A test against a recorded virtual keyboard shows the release lifts Shift and the button and leaves Caps Lock locked. That change, in 1869135, came after the live run and is covered by unit tests only.
  • scripts/check.sh passes.

This touches the session loop in crates/gliff-server/src/session.rs, and conflicts with #57, #58, #64, #73, #80 and #81 there, and with #47. I have marked it as a draft until those settle, and I will rebase it then.

Opus 5.5 high, on behalf of TechLuddite.
Implemented with Claude Opus 5.5 at high effort and reviewed by GPT-6.1 (Codex) at high reasoning effort over five passes. A re-review on 2026-10-19 by Claude, GPT-6.1 and Grok 4.7 at medium effort found the Caps Lock reset and the untested deadline arm, both fixed in 1869135, and GPT-6.1 and Grok 4.7 reviewed that fix.

The server noticed EOF and write errors, but not a link that goes quiet.
It sent one Ping at startup, the session loop had no read deadline, and
gliff sets no ssh keepalive. A client machine that slept or lost its
network with a key held left that key pressed on the server until the
transport failed, which has no fixed bound.

The session now pings every 2 s on a monotonic interval, and any client
message counts as liveness. When none arrives within 10 s of a Ping
finishing its write, or within 30 s while writes are blocked and no Ping
has been written, the session sends ReleaseAll and drops key and pointer
input until the client answers a Ping sent after that point. A client
that stays silent ends the session through the normal loop exit, so the
ordered teardown runs: after 5 minutes on a headless output, which a
reconnect creates anew rather than reattaching, or 60 s on a mirrored
one, which holds nothing the user loses.

One Heartbeat is shared by the reader task, the writer task and the
session loop, which all run on the server's LocalSet. The reader applies
each message as it reads it and matches a Pong there, and the writer
records each Ping when its write completes, so events reach the
heartbeat in the order they happened. The session's timer only checks
the deadline, and a backlog of slow handlers (a resize waits on a mode
change) can neither postpone the release nor hide a recovery. The reader
tags each message with the input epoch it arrived in, which changes on
every loss, so input received while lost, or received before a release
but handled after it, is never injected.

Clearing the outstanding tokens on loss keeps a buffered Pong for an
earlier Ping from restoring input, and tokens only grow, so a wall clock
step cannot reuse one. The writer queue now accepts a Ping, coalescing
an unwritten one with the newer one. The protocol is unchanged: clients
since v0.3.0 already answer every Ping.
stream-bench and serve-test answered ServerMsg::Ping only before the
StreamConfig, so under the server heartbeat a probe client that paused
and resumed never answered a post-loss Ping and its input stayed
dropped. Both loops now reply with Pong while streaming, as the gliff
client does, so the probe behaves like a real client.
The heartbeat's mid-session release sent ReleaseAll, which zeroes the
modifier mask and resets the XKB state. That is right at teardown, but a
client that stalled for 10 s and came back found Caps Lock off and the
layout back at group 0 on the remote for the rest of the session.

A new InputCmd::ReleaseHeld lets go of each pressed key through
KeyState::update, sending the modifiers that returns, and of each
pressed button. Teardown still uses ReleaseAll.

The session loop's deadline arm is now heartbeat_due, which sleeps to
the deadline and checks it on tokio's clock, so a test can run it on
the paused clock. Unpaused, tokio's clock is the std one. tokio's
test-util is a gliff-server dev-dependency, as in gliff-transport.
@TechLuddite
TechLuddite marked this pull request as draft October 19, 2026 20:50
@TechLuddite

Copy link
Copy Markdown
Contributor Author

Two points on the heartbeat commits pushed on 2026-10-22 (9ec6bc6, ae6c8fa, 4842dd7). The commit bodies are empty, so I may be misunderstanding the reason for the gate.

  1. The feature gate leaves held keys stuck for older clients. The 0.4.0 client answers every Ping, both in the handshake and in the session loop (crates/gliff/src/net.rs:326 and :712 at 089662f), so it already does what the heartbeat needs. The macOS client in Add native macOS client and host with LAN discovery #47 answers every Ping too, and sends its own Ping every 5 s (crates/gliff-macos/src/client.rs:229 at 4e992fc), but its FEATURES list is empty. With the gate, neither client ever gets held input released when it goes silent, which is the case F7 is about. I suggest we send the periodic Pings and run the release and recovery for every client, and gate only the end of the session on heartbeat. Older clients then keep their original session lifetime, and a silent one still has its keys released. If you know of a client that answers Pings too late for the 10 s deadline, that would change this, and I would like to hear which one.

  2. One yield_now does not make sure the reader has applied a waiting Pong after a stall. In tokio 1.53.1, yield_now defers its wake until after the I/O driver is polled (runtime/scheduler/current_thread/mod.rs:475), and that same driver poll is what wakes the reader on the TCP transport. LocalSet::run_until then polls the session future before it runs local tasks (task/local.rs:1073 and :1077), so Heartbeat::check can run before the spawned reader (crates/gliff-server/src/session.rs:494) has applied the Pong. Stdin is covered when its blocking read finished during the stall, but a Pong that arrives split across two reads can still be missed. The new test spawns a task that is ready at once, so it does not exercise the socket or the framing. I suggest we remove the stall at its source by moving the synchronous compositor calls in the session loop, such as self.instance.monitors() in wait_for_mode (session.rs:851), off the runtime thread with spawn_blocking, if the IPC handle allows it. If that is too wide for this PR, a second yield_now before check covers the TCP case, but it relies on a poll order that tokio documents as not guaranteed (task/yield_now.rs:24-32), so it should come with a test that writes the Pong into a real socket.

Can we agree on how to handle each point? I can push the changes to this branch.

Opus 5.5 high, on behalf of TechLuddite.
Reviewed with Claude Opus 5.5 at high effort, with both points checked by GPT-6.1 (Codex) and Grok 4.7, both at high reasoning effort.

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.

2 participants