Public Access
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.
364 lines
13 KiB
C#
364 lines
13 KiB
C#
using DodoSSH.Client.Ssh;
|
|
|
|
namespace DodoSSH.Client.Terminal.Tests;
|
|
|
|
/// <summary>A shell session that produces output on demand, for exercising the pump.</summary>
|
|
internal sealed class FakeShellSession : ISshShellSession
|
|
{
|
|
private readonly List<byte> written = [];
|
|
private readonly Lock gate = new();
|
|
|
|
private readonly bool blockReads;
|
|
private readonly TimeSpan? pacePerRead;
|
|
|
|
private long remaining;
|
|
private byte pattern;
|
|
|
|
/// <param name="bytesToProduce">
|
|
/// How many bytes to emit before reporting end of stream. <see cref="long.MaxValue"/> for an
|
|
/// endless producer, which is what a runaway remote process looks like — those sessions are ended
|
|
/// by disposing the pump rather than by running out of data.
|
|
/// </param>
|
|
/// <param name="blockReads">
|
|
/// True for a shell that is open and live but has nothing to say — an idle prompt, rather than either
|
|
/// end of the "produces bytes" and "hit end of stream" spectrum <paramref name="bytesToProduce"/>
|
|
/// covers. <see cref="ReadAsync"/> then blocks until cancelled, which is what a real idle SSH channel's
|
|
/// read does. Exists for tests that need a session whose <c>Run</c> stays live without a background
|
|
/// read loop racing the test for control of the pump's credit window — see the reattach tests in
|
|
/// <c>TerminalWorkspaceTests</c>.
|
|
/// </param>
|
|
/// <param name="pacePerRead">
|
|
/// How long each read takes, or null for one that returns as fast as the CPU can fill a buffer.
|
|
/// </param>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// The default producer emits at memory bandwidth, which is faster than any network and faster than
|
|
/// the pump can frame and send — useful for the throughput harness, and useless for asking whether a
|
|
/// terminal that <em>can</em> keep up loses anything, because nothing can keep up with it. A pace makes
|
|
/// the fake a link instead of a memcpy: 8 KiB every millisecond is 8 MB/s, which is a fast remote
|
|
/// rather than an impossible one.
|
|
/// </para>
|
|
/// </remarks>
|
|
internal FakeShellSession(
|
|
long bytesToProduce = 0,
|
|
bool blockReads = false,
|
|
TimeSpan? pacePerRead = null)
|
|
{
|
|
remaining = bytesToProduce;
|
|
this.blockReads = blockReads;
|
|
this.pacePerRead = pacePerRead;
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public bool IsOpen { get; private set; } = true;
|
|
|
|
/// <inheritdoc />
|
|
/// <remarks>
|
|
/// Settable, because it is the whole of what tells a shell that exited apart from one whose connection
|
|
/// broke — a test sets it for the second case and leaves it null for the first, which is exactly what the
|
|
/// real session's latch does off SSH.NET's error event.
|
|
/// </remarks>
|
|
public string? TransportFailure { get; set; }
|
|
|
|
/// <summary>Reads issued against this session, to detect a pump that kept reading.</summary>
|
|
public int ReadCount { get; private set; }
|
|
|
|
/// <summary>Bytes the pump wrote toward the remote.</summary>
|
|
public byte[] Written
|
|
{
|
|
get
|
|
{
|
|
lock (gate)
|
|
{
|
|
return [.. written];
|
|
}
|
|
}
|
|
}
|
|
|
|
/// <summary>The last size the pump forwarded, or null if it forwarded none.</summary>
|
|
public TerminalSize? LastResize { get; private set; }
|
|
|
|
/// <summary>How many resizes were forwarded, so a dropped one is observable.</summary>
|
|
public int ResizeCount { get; private set; }
|
|
|
|
/// <inheritdoc />
|
|
/// <remarks>
|
|
/// Returns 0 once the configured budget is spent, which is what a remote closing the channel looks
|
|
/// like. Not synchronous: a fake that never yields would let the pump's read loop monopolise the
|
|
/// thread and hide any ordering problem between reading and flushing.
|
|
/// </remarks>
|
|
public async ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken)
|
|
{
|
|
ReadCount++;
|
|
|
|
if (blockReads)
|
|
{
|
|
// Never completes on its own. The only way out is the same way a real blocked read ends: the
|
|
// token being cancelled, which is what disposing the pump does.
|
|
await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken).ConfigureAwait(false);
|
|
}
|
|
|
|
if (pacePerRead is { } pace)
|
|
{
|
|
await Task.Delay(pace, cancellationToken).ConfigureAwait(false);
|
|
}
|
|
|
|
await Task.Yield();
|
|
cancellationToken.ThrowIfCancellationRequested();
|
|
|
|
if (remaining <= 0)
|
|
{
|
|
return 0;
|
|
}
|
|
|
|
var count = (int)Math.Min(buffer.Length, remaining);
|
|
buffer.Span[..count].Fill(unchecked(pattern++));
|
|
remaining -= count;
|
|
|
|
return count;
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask WriteAsync(ReadOnlyMemory<byte> data, CancellationToken cancellationToken)
|
|
{
|
|
lock (gate)
|
|
{
|
|
written.AddRange(data.ToArray());
|
|
}
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public void Resize(TerminalSize size)
|
|
{
|
|
// Mirrors the real session: an unusable size is dropped rather than forwarded.
|
|
if (!size.IsUsable)
|
|
{
|
|
return;
|
|
}
|
|
|
|
ResizeCount++;
|
|
LastResize = size;
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask DisposeAsync()
|
|
{
|
|
IsOpen = false;
|
|
remaining = 0;
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Hands out <see cref="FakeShellSession"/>s, so a workspace can be driven with no network.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// Exists for the session-lifetime tests. Everything else in this suite works on a pump directly; the
|
|
/// workspace is the layer that decides when a session is over, and that decision is what needs a
|
|
/// connection whose shell can be made to end on cue.
|
|
/// </remarks>
|
|
internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue, bool blockShellReads = false)
|
|
: ISshConnectionFactory
|
|
{
|
|
/// <summary>Armed on every shell handed out, so a workspace-level test can produce a dropped connection.</summary>
|
|
internal string? TransportFailure { get; set; }
|
|
|
|
/// <summary>Connections handed out, in order.</summary>
|
|
internal List<FakeConnection> Connections { get; } = [];
|
|
|
|
/// <inheritdoc />
|
|
public Task<ISshConnection> ConnectAsync(
|
|
SshConnectionRequest request,
|
|
IProgress<SshConnectionPhase>? progress,
|
|
CancellationToken cancellationToken)
|
|
{
|
|
// The real factory's own order, so that a test watching this fake is watching the same sequence a
|
|
// real handshake produces. It cannot report CheckingHostKey — there is no key exchange here to
|
|
// produce a key — and inventing one would make this the only place that phase came from.
|
|
progress?.Report(SshConnectionPhase.Reaching);
|
|
progress?.Report(SshConnectionPhase.Authenticating);
|
|
|
|
var connection = new FakeConnection(request, bytesPerShell, blockShellReads)
|
|
{
|
|
TransportFailure = TransportFailure,
|
|
};
|
|
|
|
Connections.Add(connection);
|
|
|
|
return Task.FromResult<ISshConnection>(connection);
|
|
}
|
|
}
|
|
|
|
/// <summary>A connection that opens fake shells and records its own disposal.</summary>
|
|
internal sealed class FakeConnection(SshConnectionRequest request, long bytesPerShell, bool blockShellReads = false)
|
|
: ISshConnection
|
|
{
|
|
/// <inheritdoc />
|
|
public bool IsConnected { get; private set; } = true;
|
|
|
|
/// <inheritdoc />
|
|
public HostKeyPresentation HostKey { get; } =
|
|
new(request.Host, request.Port, "ssh-ed25519", "SHA256:fake");
|
|
|
|
/// <inheritdoc />
|
|
public string Cipher { get; } = "aes256-gcm@openssh.com";
|
|
|
|
/// <summary>The shell this connection opened, if it opened one.</summary>
|
|
internal FakeShellSession? Shell { get; private set; }
|
|
|
|
/// <inheritdoc cref="ISshShellSession.TransportFailure" />
|
|
/// <remarks>Handed to whatever shell this connection opens, so the arming happens before the pump runs.</remarks>
|
|
internal string? TransportFailure { get; init; }
|
|
|
|
/// <summary>Whether the connection was disposed, which is what closing a session must do.</summary>
|
|
internal bool IsDisposed { get; private set; }
|
|
|
|
/// <inheritdoc />
|
|
public Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken)
|
|
{
|
|
Shell = new FakeShellSession(bytesPerShell, blockShellReads) { TransportFailure = TransportFailure };
|
|
|
|
return Task.FromResult<ISshShellSession>(Shell);
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask DisposeAsync()
|
|
{
|
|
IsDisposed = true;
|
|
IsConnected = false;
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Counts output without keeping it, and acknowledges it as a renderer that is keeping up would.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <see cref="RecordingTransport"/>'s cheap sibling, and the throughput harness needs it to be the one
|
|
/// measuring rather than the one measured: recording copies every frame into a list that grows to the size
|
|
/// of the whole run, which at the rates that harness drives makes the <em>test double</em> the slowest thing
|
|
/// in the pipeline. A harness whose consumer cannot keep up is measuring itself.
|
|
/// </remarks>
|
|
internal sealed class CountingTransport : ITerminalTransport
|
|
{
|
|
private long outputBytes;
|
|
|
|
/// <summary>Set to acknowledge every output frame immediately, as a keeping-up renderer would.</summary>
|
|
internal TerminalSessionPump? AutoAcknowledge { get; set; }
|
|
|
|
/// <summary>Payload bytes of every output frame sent so far.</summary>
|
|
internal long OutputBytes => Interlocked.Read(ref outputBytes);
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask SendAsync(ReadOnlyMemory<byte> frame, CancellationToken cancellationToken)
|
|
{
|
|
if (TerminalFrame.TryRead(frame.Span, out var opcode, out _, out var payload)
|
|
&& opcode == (byte)TerminalServerOpcode.Output)
|
|
{
|
|
_ = Interlocked.Add(ref outputBytes, payload.Length);
|
|
|
|
AutoAcknowledge?.Acknowledge((uint)payload.Length);
|
|
}
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
}
|
|
|
|
/// <summary>Records frames, and can acknowledge them to keep credit flowing.</summary>
|
|
internal sealed class RecordingTransport : ITerminalTransport
|
|
{
|
|
private readonly List<byte[]> frames = [];
|
|
private readonly Lock gate = new();
|
|
|
|
/// <summary>Set to acknowledge every output frame immediately, as a keeping-up renderer would.</summary>
|
|
internal TerminalSessionPump? AutoAcknowledge { get; set; }
|
|
|
|
/// <summary>Frames sent so far.</summary>
|
|
internal IReadOnlyList<byte[]> Frames
|
|
{
|
|
get
|
|
{
|
|
lock (gate)
|
|
{
|
|
return [.. frames];
|
|
}
|
|
}
|
|
}
|
|
|
|
/// <inheritdoc />
|
|
public ValueTask SendAsync(ReadOnlyMemory<byte> frame, CancellationToken cancellationToken)
|
|
{
|
|
var copy = frame.ToArray();
|
|
|
|
lock (gate)
|
|
{
|
|
frames.Add(copy);
|
|
}
|
|
|
|
if (AutoAcknowledge is { } pump
|
|
&& TerminalFrame.TryRead(copy, out var opcode, out _, out var payload)
|
|
&& opcode == (byte)TerminalServerOpcode.Output)
|
|
{
|
|
pump.Acknowledge((uint)payload.Length);
|
|
}
|
|
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
|
|
/// <summary>Concatenated payloads of every output frame.</summary>
|
|
internal byte[] OutputBytes()
|
|
{
|
|
var output = new List<byte>();
|
|
|
|
foreach (var frame in Frames)
|
|
{
|
|
if (TerminalFrame.TryRead(frame, out var opcode, out _, out var payload)
|
|
&& opcode == (byte)TerminalServerOpcode.Output)
|
|
{
|
|
output.AddRange(payload);
|
|
}
|
|
}
|
|
|
|
return [.. output];
|
|
}
|
|
|
|
/// <summary>Concatenated payloads of every frame of one opcode.</summary>
|
|
internal byte[] PayloadsOf(TerminalServerOpcode opcode)
|
|
{
|
|
var payloads = new List<byte>();
|
|
|
|
foreach (var frame in Frames)
|
|
{
|
|
if (TerminalFrame.TryRead(frame, out var actual, out _, out var payload)
|
|
&& actual == (byte)opcode)
|
|
{
|
|
payloads.AddRange(payload);
|
|
}
|
|
}
|
|
|
|
return [.. payloads];
|
|
}
|
|
|
|
/// <summary>Counts frames of one opcode.</summary>
|
|
internal int CountOf(TerminalServerOpcode opcode) =>
|
|
Frames.Count(frame =>
|
|
TerminalFrame.TryRead(frame, out var actual, out _, out _)
|
|
&& actual == (byte)opcode);
|
|
}
|
|
|
|
/// <summary>An <see cref="IProgress{T}"/> that runs its callback on the thread that reported.</summary>
|
|
/// <remarks>
|
|
/// <c>System.Progress<T></c> would post to a captured synchronisation context, or to the thread pool
|
|
/// when there is none — which is what a test has here — so a list it appended to would be asserted on before
|
|
/// it had been written. This is the same reason the shell does not use it either; see
|
|
/// <c>VaultViewModel.ReporterFor</c>.
|
|
/// </remarks>
|
|
internal sealed class DelegateProgress<T>(Action<T> report) : IProgress<T>
|
|
{
|
|
public void Report(T value) => report(value);
|
|
}
|