Files
DodoSSH/src/DodoSSH.Client.Terminal/OutputBacklog.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

265 lines
9.8 KiB
C#

namespace DodoSSH.Client.Terminal;
/// <summary>
/// One session's bounded queue of output waiting for the renderer, oldest first.
/// </summary>
/// <remarks>
/// <para>
/// ◆ <b>This is where a session's memory is actually bounded, and it exists because the obvious place
/// cannot do it.</b> The design used to be that <see cref="CreditWindow"/> bounded everything: the pump
/// stopped reading the SSH channel when the renderer fell behind, SSH's own receive window closed, and the
/// remote blocked on write. The middle step is not true of the library this client uses.
/// <c>Renci.SshNet.Channels.Channel.OnData</c> calls its window adjustment <em>before</em> raising
/// <c>DataReceived</c> and re-opens the channel window as soon as it drops below one packet, and
/// <c>ShellStream</c>'s own handler copies the bytes into a growable buffer without blocking and without a
/// ceiling. So a reader that stops reading does not slow the remote down by one byte; it only decides
/// whose buffer the flood accumulates in — and SSH.NET's grows by doubling and does not give the memory
/// back until the stream is disposed. A remote running <c>yes</c> took the process with it.
/// </para>
/// <para>
/// So the pump now reads continuously, which is what keeps SSH.NET's buffer empty, and the backlog it
/// reads into is this one, with a hard capacity. When the renderer cannot keep up the <em>oldest</em>
/// bytes go rather than the newest, because the newest are the ones somebody is waiting to see: the tail
/// of a flood is the prompt that comes back after it. Dropping is announced rather than silent — see
/// <see cref="Take"/> — and it is the honest end of the trade, since output arriving faster than the
/// terminal can draw it was going to scroll off a 5,000-line scrollback regardless.
/// </para>
/// <para>
/// The credit window is still here and still does its own job: it bounds what is in flight <em>to the
/// renderer</em>, which is the buffer inside the WebView that nothing on this side can see. What changed
/// is that it no longer pretends to bound what is in flight from the remote.
/// </para>
/// </remarks>
public sealed class OutputBacklog
{
/// <summary>
/// Default capacity: 4 MiB.
/// </summary>
/// <remarks>
/// Sixteen times the credit window, so nothing an ordinary burst produces is ever dropped — a
/// screenful of dense output is kilobytes, and a build that scrolls for a second is well under this.
/// It is a per-session cost, and it is the whole of what a runaway remote can take, which is the
/// number worth choosing against: twenty tabs all flooding at once is 80 MiB and survivable, where the
/// same twenty unbounded was whatever the machine had.
/// </remarks>
public const int DefaultCapacityBytes = 4 * 1024 * 1024;
private readonly Lock gate = new();
private readonly Queue<byte[]> segments = new();
private readonly int capacityBytes;
/// <summary>Signalled when output arrives at an empty backlog, and when it is completed.</summary>
private TaskCompletionSource available = CreateSignal();
/// <summary>How much of the head segment has already been taken.</summary>
private int head;
private int buffered;
private long droppedSinceTake;
private long droppedTotal;
private bool completed;
/// <param name="capacityBytes">How many unsent bytes to hold before dropping the oldest.</param>
public OutputBacklog(int capacityBytes = DefaultCapacityBytes)
{
ArgumentOutOfRangeException.ThrowIfLessThan(capacityBytes, 1);
this.capacityBytes = capacityBytes;
}
/// <summary>How many unsent bytes are held.</summary>
public int Buffered
{
get
{
lock (gate)
{
return buffered;
}
}
}
/// <summary>How many bytes this backlog has discarded over its whole life.</summary>
public long DroppedTotal
{
get
{
lock (gate)
{
return droppedTotal;
}
}
}
/// <summary>Whether the producer has said there will be no more output.</summary>
public bool IsCompleted
{
get
{
lock (gate)
{
return completed;
}
}
}
/// <summary>
/// Adds output, dropping the oldest already held if that puts the backlog over capacity.
/// </summary>
/// <remarks>
/// Takes ownership of <paramref name="segment"/>; the caller must not write to it afterwards.
/// Appending to a completed backlog does nothing rather than throwing, because a producer finding out
/// it has been shut down is a race rather than a mistake.
/// </remarks>
/// <param name="segment">Bytes read from the remote, in order.</param>
public void Append(byte[] segment)
{
ArgumentNullException.ThrowIfNull(segment);
if (segment.Length == 0)
{
return;
}
lock (gate)
{
if (completed)
{
return;
}
var wasEmpty = buffered == 0;
segments.Enqueue(segment);
buffered += segment.Length;
// Never the segment just added, however large it is. A read bigger than the whole capacity is
// possible if somebody configures one, and a backlog that answered every take with nothing
// would stall the session rather than bound it.
while (buffered > capacityBytes && segments.Count > 1)
{
var oldest = segments.Dequeue();
var discarded = oldest.Length - head;
head = 0;
buffered -= discarded;
droppedSinceTake += discarded;
droppedTotal += discarded;
}
// Only on the transition, because a waiter that found bytes here never captured the signal in
// the first place — see WaitForOutputAsync, which checks under this same lock. Signalling on
// every append would allocate a TaskCompletionSource per read of a session doing megabytes a
// second, for a waiter that is not there.
if (wasEmpty)
{
Signal();
}
}
}
/// <summary>
/// Moves the oldest output held into <paramref name="destination"/>, as much of it as will fit.
/// </summary>
/// <remarks>
/// <para>
/// Into a caller's span rather than into an array of its own, because this is the hot path: the pump
/// writes straight into the frame it is about to send, so a terminal showing a gigabyte costs one copy
/// of it here rather than two.
/// </para>
/// <para>
/// <paramref name="dropped"/> is what makes a drop visible instead of silent. It counts the bytes
/// discarded since the previous take, which is exactly the gap sitting immediately before the bytes
/// being handed over now. The caller writes a line saying so, and it has to — the alternative is a
/// terminal quietly showing a different transcript from the one the remote sent. It is reported even
/// when nothing is taken, so a drop at the very end of a session is still announced.
/// </para>
/// </remarks>
/// <param name="destination">Where the bytes go, oldest first.</param>
/// <param name="dropped">Bytes discarded since the previous take, and zero on the ordinary path.</param>
/// <returns>How many bytes were written, which is zero when nothing is held.</returns>
public int TakeInto(Span<byte> destination, out long dropped)
{
lock (gate)
{
dropped = droppedSinceTake;
droppedSinceTake = 0;
var take = Math.Min(destination.Length, buffered);
var offset = 0;
while (offset < take)
{
var oldest = segments.Peek();
var fromThis = Math.Min(oldest.Length - head, take - offset);
oldest.AsSpan(head, fromThis).CopyTo(destination[offset..]);
offset += fromThis;
head += fromThis;
buffered -= fromThis;
if (head == oldest.Length)
{
_ = segments.Dequeue();
head = 0;
}
}
return take;
}
}
/// <summary>Says there will be no more output, releasing anything waiting for some.</summary>
public void Complete()
{
lock (gate)
{
completed = true;
Signal();
}
}
/// <summary>Waits until there is output to take, or until there will never be any.</summary>
/// <param name="cancellationToken">Abandons the wait.</param>
/// <returns><see langword="false"/> once the backlog is completed and empty.</returns>
public async ValueTask<bool> WaitForOutputAsync(CancellationToken cancellationToken)
{
while (true)
{
Task signal;
lock (gate)
{
if (buffered > 0)
{
return true;
}
if (completed)
{
return false;
}
signal = available.Task;
}
await signal.WaitAsync(cancellationToken).ConfigureAwait(false);
}
}
/// <remarks>
/// Completed inside the lock so a waiter cannot miss the transition, and replaced there too so the
/// next wait is on a signal that has not already fired. Continuations run asynchronously, so nothing
/// resumes while the lock is held.
/// </remarks>
private void Signal()
{
_ = available.TrySetResult();
available = CreateSignal();
}
private static TaskCompletionSource CreateSignal() =>
new(TaskCreationOptions.RunContinuationsAsynchronously);
}