Files
DodoSSH/src/DodoSSH.Client.Terminal/TerminalFrame.cs
T
jaap-jan 9ead42171c
ci / build and test (push) Successful in 3m12s
ci / desktop nightly (push) Successful in 1m16s
ci / api image (push) Successful in 45s
ci / android head (push) Successful in 7m11s
Stop a flooding remote growing the process, and let an ended session go
The terminal's memory was bounded by an argument that is not true of the SSH
library underneath it, and the whole of CreditWindow's remark was that argument:
the pump stops reading when the renderer falls behind, SSH's own receive window
closes, the remote sshd blocks on write, and the backpressure reaches the process
producing the output. The middle step does not happen. Renci.SshNet's
Channels.Channel.OnData calls its window adjustment before it raises DataReceived
and re-opens the channel window as soon as it drops below one packet, and
ShellStream's own handler copies the bytes into a System.Net.ArrayBuffer without
blocking and without a ceiling — a buffer that doubles on demand and is not given
back until the stream is disposed. So a reader that stopped reading slowed the
remote down by nothing at all. It only chose whose buffer the flood landed in,
and it chose the one nothing bounds.

Measured against the container this repository already runs sshd in: a reader
that reads nothing while `yes` runs leaves ShellStream holding 8.9 MB after one
second, 26.8 MB after three and 44.3 MB after five, its capacity stepping 16 → 32
→ 64 MB. That is the report of "a massive memory leak on Windows"; it is not a
Windows fault, it is simply where long sessions with heavy output live.

The fix is to read continuously, which is what keeps SSH.NET's buffer empty, and
to put the bound where it can actually be enforced. OutputBacklog is that bound:
a byte-capped queue of unsent output, 4 MiB by default, which drops its oldest
bytes when full and counts what it discarded. Oldest rather than newest, because
the tail of a flood is the prompt somebody is waiting for.

The two bounds are now separate and each is enforced where it means something.
The backlog bounds what is waiting to be sent. The credit window goes on bounding
what has been sent and not yet drawn — the queue inside the WebView that nothing
on this side can measure — and is reserved against in the flush loop rather than
the read loop. Reading is part of neither, because reading was never what the
remote was waiting on.

Dropping is announced rather than silent. A terminal quietly showing a different
transcript from the one the remote sent is worse than one that says it could not
keep up, because the second can be believed. The line goes out under its own
opcode, TerminalServerOpcode.OutputDropped, rather than spliced into the output
stream: what this application says about the transcript is not part of the
transcript, and keeping them apart is also what makes the arithmetic hold — every
byte read is either in an Output payload or counted in BytesDropped. It opens
with an SGR reset, because the cut lands at an arbitrary offset and whatever
escape sequence the remote was in the middle of has been left half-said.

Frames are now capped at 64 KiB and taken straight into the frame buffer. The cap
is the large-object-heap threshold rather than a guess — a quarter-megabyte array
allocated sixty times a second is collected only by a generation 2 pass — and the
direct take removes an allocation and a copy of every byte the terminal shows.

The second half is the one that made the first half's cost persist. Nothing gave
a session back when the remote hung up: TerminalWorkspace kept the entry, and
with it an undisposed ISshConnection — an authenticated socket, its cipher state,
and a keep-alive putting a packet on the wire every thirty seconds — plus the
pump and SSH.NET's read buffer at whatever size the session's busiest moment had
grown it to, for as long as the tab stayed open. A tab somebody typed `exit` into
an hour ago was indistinguishable from a live one in everything except what it
drew. ReleaseAsync disposes both, from AnnounceEndedAsync and after the
announcement: the tab is told first, because tearing down an SSH connection in
front of that would be latency on the one thing the user is watching for. The
dictionary entry stays, because LiveSessionCount, IsSessionLive and the tab's own
ENDED and RECONNECT all read it. SendInputAsync therefore asks about liveness
rather than presence, since the shell channel is gone the moment the remote is.

Two faults introduced on the way here and fixed, both worth naming because
neither would have been visible from the outside:

The read loop left through a condition on its cancellation token, and RunAsync
reads a read loop that returns as the remote having closed the channel. A tab the
user closed would have reported itself as a shell that exited on its own. It
throws instead.

With the reader finished and a renderer that had stopped acknowledging, the flush
loop span: output to send, no credit to send it with, and a wait for more output
that returned at once because there was some. RunAsync never completed, so the tab
was never told and the connection was never released — a session invisible and
permanent. The loop now leaves for the drain as soon as the backlog is complete,
and RunAsync resets the credit window alongside completing it so a flusher already
parked on an acknowledgement that will never come is released.

Four tests asserted the old contract and now assert its reverse, which is the
point rather than a relaxation — WhenCreditRunsOut_ThePumpStopsReading is now
WhenCreditRunsOut_ThePumpKeepsReadingAndTheBacklogHoldsTheLine, and has to prove
both halves at once. The throughput harness gained a counting transport, because
a consumer that copies every frame into a 32 MB list was the slowest thing in the
pipeline and a harness whose consumer cannot keep up measures itself. The fake
shell gained a pace, so "a renderer that keeps up loses nothing" can be asked of a
producer a network could actually deliver rather than of one running at memory
bandwidth.

The test that would have caught this in the first place is
ARunawayRemote_KeepsBeingReadAndStaysInsideTheBacklog, and it needs the real
SSH.NET and the real sshd to say anything: a fake shell hands over bytes when it
is asked, and a real channel hands them over when they arrive. Under the old code
its BytesRead would have stopped at exactly WindowBytes.

121 terminal tests pass over three consecutive runs with no flakes, 105 against
the container, and the app, session, layout and remaining suites are unchanged and
green. Manual checks 1.10a and 1.10b cover the two symptoms a person can see: a
working set that plateaus under `yes`, and a connection that goes when the shell
does rather than when the tab does.
2026-09-04 17:16:23 +02:00

401 lines
18 KiB
C#

using System.Buffers.Binary;
namespace DodoSSH.Client.Terminal;
/// <summary>Frames the host sends to the renderer.</summary>
public enum TerminalServerOpcode : byte
{
/// <summary>Not a legal value.</summary>
Unspecified = 0,
/// <summary>Terminal output. Payload is raw bytes for <c>term.write</c>.</summary>
Output = 1,
/// <summary>A session has been created; the renderer should attach a terminal to it.</summary>
SessionOpened = 2,
/// <summary>A session has ended. Payload is a UTF-8 reason for the user.</summary>
SessionClosed = 3,
/// <summary>
/// This session's pane is the one to show. No payload.
/// </summary>
/// <remarks>
/// <para>
/// Every session gets a pane and all but one are hidden, so something has to say which. The page picked
/// the newest on its own, which is right exactly once — at the moment a session opens — and wrong for
/// every tab switch afterwards, because the page has no idea a tab strip exists. It is the host that
/// knows which tab is selected, so it is the host that says.
/// </para>
/// <para>
/// An opcode rather than script evaluation through the WebView bridge, which would have been the smaller
/// change. The bridge is UI-thread-bound and unordered with respect to this socket: a switch sent that
/// way could arrive before the <see cref="SessionOpened"/> frame for the pane it names, and the page
/// would activate a session that does not exist yet. On this socket the two are ordered by construction.
/// </para>
/// </remarks>
SessionActivated = 4,
/// <summary>
/// This session is gone for good; the renderer should destroy its pane. No payload.
/// </summary>
/// <remarks>
/// <para>
/// Deliberately not the same thing as <see cref="SessionClosed"/>. That one means the shell ended and
/// the pane must <em>stay</em> — the user was probably reading the last thing the remote said, and that
/// is usually why it ended. This one means the user closed the tab, so there is nothing left to read.
/// </para>
/// <para>
/// It exists because a pane is expensive and the page never reclaimed one. Every closed tab used to
/// leave an xterm instance, its scrollback and a WebGL context behind — and WebGL contexts are a hard
/// browser limit of about sixteen, so a working day of opening and closing terminals ends with panes
/// that cannot get a renderer. Nothing surfaced that, because the leak is inside the WebView.
/// </para>
/// </remarks>
SessionRemoved = 5,
/// <summary>
/// Text to insert into this session, as if it had been pasted.
/// </summary>
/// <remarks>
/// <para>
/// Payload is one flag byte — non-zero to press Enter after the text — followed by UTF-8.
/// </para>
/// <para>
/// <b>Through the renderer rather than straight into the input stream, and that is the whole reason this
/// opcode exists.</b> Writing the bytes to the pump would have been fewer lines and is wrong: xterm.js
/// watches the remote for <c>\e[?2004h</c> and wraps pasted text in bracketed-paste markers when the mode
/// is on, which is what makes a shell treat embedded newlines as text instead of as "run this". The host
/// process cannot do that — <c>TerminalDataPlane</c> moves opaque bytes and never parses output — so it
/// would have to guess, and guessing wrong executes every line of a multi-line snippet.
/// </para>
/// <para>
/// The flag is separate from the text for the same reason. A trailing newline inside the payload would be
/// wrapped along with everything else and arrive at the shell as a literal character; the Enter has to go
/// through <c>term.input</c>, <em>outside</em> the wrapper, or nothing runs.
/// </para>
/// </remarks>
Paste = 6,
/// <summary>
/// What size every pane should draw at. Payload is one byte, the size in CSS pixels.
/// </summary>
/// <remarks>
/// <para>
/// Not per session, although the frame carries a session id as every frame does: a font size is a
/// preference about reading rather than a property of one remote, and somebody who makes the text
/// bigger means all of it. The id is sent as zero and ignored.
/// </para>
/// <para>
/// <b>Changing it reflows the remote, and that is the point.</b> The page refits after applying, which
/// changes the column and row count and sends <see cref="TerminalClientOpcode.Resize"/> back — so a
/// larger font is a terminal with fewer columns, and the far end is told about it. That is why this is
/// a font size rather than a zoom: zoom scales pixels and leaves the remote wrapping to a width that
/// is no longer on screen, which is what made the phone's pinch-zoom worse than useless.
/// </para>
/// </remarks>
FontSize = 7,
/// <summary>
/// The system clipboard's text, for this session's pane to paste. Payload is UTF-8.
/// </summary>
/// <remarks>
/// <para>
/// Sent in answer to <see cref="TerminalClientOpcode.ClipboardPaste"/>, which is the only thing that
/// asks for it: a right-click over a pane with nothing selected. The host reads the clipboard because
/// the page cannot — see that opcode for why not.
/// </para>
/// <para>
/// <b>Not <see cref="Paste"/>, and the difference is the bracketing.</b> That opcode carries a snippet
/// off the sidebar, and the page deliberately <em>types</em> a single-line one rather than bracketing
/// it, because bash marks a bracketed paste as an active region and a snippet should read as though it
/// had been typed. A clipboard paste is the case those markers are for — readline uses them to decide it
/// has been pasted into — so this one is always bracketed. One opcode for both would need a flag
/// meaning "which of the two kinds of paste is this", which is the same distinction spelled less
/// clearly, on a payload that already has a flag byte meaning something else.
/// </para>
/// <para>
/// No flag byte, therefore, and no Enter: a clipboard paste inserts and stops. Pressing Enter for
/// somebody is what the sidebar's RUN does, and it is a thing they asked for by name.
/// </para>
/// </remarks>
Clipboard = 8,
/// <summary>
/// A line for the pane saying that output was dropped. Payload is UTF-8, already styled.
/// </summary>
/// <remarks>
/// <para>
/// Written into the pane exactly where it arrives, which is where the gap is — the socket keeps order
/// within a session, so this frame lands between the bytes before the drop and the bytes after it.
/// </para>
/// <para>
/// <b>Its own opcode rather than a few bytes spliced into an <see cref="Output"/> frame</b>, which is
/// what it was first, and the reason is that the two are not the same kind of thing. Output is what the
/// remote said, and this is what this application says about it. Keeping them apart means the host's
/// own sentence is never mistaken for the transcript — by anything reading a frame, and in particular
/// by the arithmetic that has to hold if dropping is to stay honest: every byte read from the remote is
/// either in an <see cref="Output"/> payload or counted in <c>TerminalSessionPump.BytesDropped</c>, and
/// a notice inside the output stream would put this process's words on the left of that equation.
/// </para>
/// <para>
/// Not acknowledged, and so not part of the credit window: credit accounts for what the remote sent
/// and what the renderer has drawn of it. A notice is a few dozen bytes the host chose to add, and
/// billing them to a window that is meant to describe the remote would make the number mean two
/// things.
/// </para>
/// </remarks>
OutputDropped = 9,
}
/// <summary>Frames the renderer sends to the host.</summary>
public enum TerminalClientOpcode : byte
{
/// <summary>Not a legal value.</summary>
Unspecified = 0,
/// <summary>Keystrokes. Payload is raw bytes for the remote.</summary>
Input = 1,
/// <summary>
/// Bytes actually rendered. Payload is a big-endian <see cref="uint"/>, returning credit.
/// </summary>
Acknowledge = 2,
/// <summary>The terminal was resized. Payload is four big-endian <see cref="ushort"/> values.</summary>
Resize = 3,
/// <summary>
/// The page asking for a different font size. Payload is one signed byte: a step, or zero to reset.
/// </summary>
/// <remarks>
/// <para>
/// A request rather than a statement, and the direction matters. The page owns the keyboard whenever a
/// terminal has focus — the host's window sees no key events at all then — so Ctrl+plus and Ctrl+minus
/// can only be <em>heard</em> there. But the size has to be remembered across launches and shown in a
/// control the page knows nothing about, so it is <em>decided</em> here. The page asks, the host
/// clamps, stores and answers with <see cref="TerminalServerOpcode.FontSize"/>.
/// </para>
/// <para>
/// Which also means one path, not two: the phone's buttons and the desktop's chords arrive at the same
/// place, and a size set by either is the size both remember.
/// </para>
/// </remarks>
FontSizeStep = 4,
/// <summary>
/// Text to put on the system clipboard. Payload is UTF-8.
/// </summary>
/// <remarks>
/// <para>
/// A right-click over a pane with something selected. What gets copied is the selection, and only the
/// page knows what that is — xterm draws it into a canvas and the host never sees a glyph of it — so it
/// is the text that travels here rather than a request for it.
/// </para>
/// <para>
/// <b>Through the host rather than through the page's own <c>navigator.clipboard</c>.</b> Writing the
/// clipboard from the page would work on Windows, where the renderer is Blink and
/// <c>clipboard-write</c> is granted to a focused document without asking. It is the <em>reading</em>
/// that cannot be done that way — see <see cref="ClipboardPaste"/> — and a clipboard that is the host's
/// in one direction and the page's in the other is two mechanisms to keep working across WebView2,
/// WebKitGTK and Android's WebView. The host has owned a clipboard delegate since the keychain grew its
/// COPY buttons; this is that one.
/// </para>
/// <para>
/// The session id is the pane the selection came from. Nothing routes on it — a clipboard is not a
/// property of one terminal — and it is carried because every frame carries one.
/// </para>
/// </remarks>
ClipboardCopy = 5,
/// <summary>
/// The page asking for the clipboard's text, to paste into this session. No payload.
/// </summary>
/// <remarks>
/// <para>
/// A right-click over a pane with nothing selected. The answer arrives as
/// <see cref="TerminalServerOpcode.Clipboard"/>, addressed to the session id this frame carried, so the
/// text lands in the pane the pointer was over rather than in whichever one the host believes is
/// selected. They agree in every arrangement that exists today, because only the active pane is
/// visible — but the page is the one that knows, so the page is the one that says.
/// </para>
/// <para>
/// <b>A request rather than the page reading the clipboard itself, and this is the half with no
/// alternative.</b> <c>navigator.clipboard.readText()</c> is gated behind the <c>clipboard-read</c>
/// permission in every Chromium, and an embedded WebView answers a permission request it has no handler
/// for by putting a prompt on the screen. Trading a context menu for a permission dialog is not the
/// change this was.
/// </para>
/// </remarks>
ClipboardPaste = 6,
}
/// <summary>
/// The wire format between the host process and the renderer page.
/// </summary>
/// <remarks>
/// <para>
/// Binary frames over a loopback WebSocket, not the WebView's JavaScript bridge. The official bridge
/// is UI-thread-bound string evaluation: at 10 MB/s in 4 KiB chunks that is roughly 2,500 script
/// evaluations per second on the thread that also has to paint, with base64's 33% overhead on top and
/// — decisively — no backpressure signal at all. A socket gives flow control for free.
/// </para>
/// <para>
/// Every frame carries a session id because one WebView hosts every terminal. A WebView2 instance is
/// a separate browser process, so one per tab would mean twenty renderer processes and hundreds of
/// megabytes for a normal working set of tabs.
/// </para>
/// <para>
/// Fixed 5-byte header, big-endian, no length prefix: WebSocket already delimits messages, so adding
/// our own length would be a second source of truth about where a frame ends.
/// </para>
/// </remarks>
public static class TerminalFrame
{
/// <summary>Opcode plus session id.</summary>
public const int HeaderLength = 1 + sizeof(uint);
/// <summary>Writes a frame into <paramref name="destination"/> and returns its length.</summary>
public static int Write(
Span<byte> destination,
byte opcode,
uint sessionId,
ReadOnlySpan<byte> payload)
{
var total = HeaderLength + payload.Length;
if (destination.Length < total)
{
throw new ArgumentException(
$"Need {total} bytes for the frame, got {destination.Length}.",
nameof(destination));
}
destination[0] = opcode;
BinaryPrimitives.WriteUInt32BigEndian(destination[1..], sessionId);
payload.CopyTo(destination[HeaderLength..]);
return total;
}
/// <summary>Allocates and writes a frame.</summary>
public static byte[] Create(byte opcode, uint sessionId, ReadOnlySpan<byte> payload)
{
var frame = new byte[HeaderLength + payload.Length];
Write(frame, opcode, sessionId, payload);
return frame;
}
/// <summary>
/// Reads a frame's header and payload.
/// </summary>
/// <remarks>
/// Returns false rather than throwing on anything malformed. These frames arrive from a WebView
/// page — a different process running code we shipped but do not control at runtime — so a bad
/// frame is untrusted input to be dropped, not an exceptional condition.
/// </remarks>
public static bool TryRead(
ReadOnlySpan<byte> frame,
out byte opcode,
out uint sessionId,
out ReadOnlySpan<byte> payload)
{
opcode = 0;
sessionId = 0;
payload = default;
if (frame.Length < HeaderLength)
{
return false;
}
opcode = frame[0];
sessionId = BinaryPrimitives.ReadUInt32BigEndian(frame[1..]);
payload = frame[HeaderLength..];
return true;
}
/// <summary>Reads an <see cref="TerminalClientOpcode.Acknowledge"/> payload.</summary>
public static bool TryReadAcknowledgement(ReadOnlySpan<byte> payload, out uint rendered)
{
rendered = 0;
if (payload.Length != sizeof(uint))
{
return false;
}
rendered = BinaryPrimitives.ReadUInt32BigEndian(payload);
return true;
}
/// <summary>Reads a <see cref="TerminalClientOpcode.Resize"/> payload.</summary>
public static bool TryReadResize(ReadOnlySpan<byte> payload, out DodoSSH.Client.Ssh.TerminalSize size)
{
size = default;
if (payload.Length != sizeof(ushort) * 4)
{
return false;
}
size = new DodoSSH.Client.Ssh.TerminalSize(
BinaryPrimitives.ReadUInt16BigEndian(payload),
BinaryPrimitives.ReadUInt16BigEndian(payload[2..]),
BinaryPrimitives.ReadUInt16BigEndian(payload[4..]),
BinaryPrimitives.ReadUInt16BigEndian(payload[6..]));
return true;
}
/// <summary>Writes a <see cref="TerminalClientOpcode.Resize"/> payload. Used by tests and tooling.</summary>
public static byte[] CreateResizePayload(DodoSSH.Client.Ssh.TerminalSize size)
{
var payload = new byte[sizeof(ushort) * 4];
BinaryPrimitives.WriteUInt16BigEndian(payload, size.Columns);
BinaryPrimitives.WriteUInt16BigEndian(payload.AsSpan(2), size.Rows);
BinaryPrimitives.WriteUInt16BigEndian(payload.AsSpan(4), size.PixelWidth);
BinaryPrimitives.WriteUInt16BigEndian(payload.AsSpan(6), size.PixelHeight);
return payload;
}
/// <summary>Reads a <see cref="TerminalClientOpcode.FontSizeStep"/> payload.</summary>
/// <remarks>
/// Signed, because the one byte carries a direction as well as a magnitude — and zero means reset,
/// which is why this cannot be a flag pair.
/// </remarks>
public static bool TryReadFontSizeStep(ReadOnlySpan<byte> payload, out int step)
{
step = 0;
if (payload.Length != 1)
{
return false;
}
step = (sbyte)payload[0];
return true;
}
/// <summary>Writes a <see cref="TerminalClientOpcode.FontSizeStep"/> payload.</summary>
public static byte[] CreateFontSizeStepPayload(int step) => [(byte)(sbyte)Math.Clamp(step, -128, 127)];
/// <summary>Writes a <see cref="TerminalServerOpcode.FontSize"/> payload.</summary>
public static byte[] CreateFontSizePayload(int pixels) => [(byte)Math.Clamp(pixels, 1, 255)];
/// <summary>Writes an <see cref="TerminalClientOpcode.Acknowledge"/> payload.</summary>
public static byte[] CreateAcknowledgementPayload(uint rendered)
{
var payload = new byte[sizeof(uint)];
BinaryPrimitives.WriteUInt32BigEndian(payload, rendered);
return payload;
}
}